Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include node_resource_group as variable #136

Merged
merged 2 commits into from
Apr 13, 2022
Merged

Conversation

afym
Copy link

@afym afym commented Dec 30, 2021

This PR close #135

@ghost
Copy link

ghost commented Dec 30, 2021

CLA assistant check
All CLA requirements met.

Copy link

@sbl-matter sbl-matter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much needed change

@andypanix
Copy link

It would also be useful for us to have this PR approved, we are using this module in a context where we have resource group name constrainments and not being able to customise it is a bit problematic.

@RobMokkink
Copy link

Like @andypanix said, please merge this feature, otherwise we have a messed up naming convention.

@inisitijitty
Copy link

inisitijitty commented Mar 22, 2022

Hey reviewers folks, can this be approved please ?

@shurik-io
Copy link

Hi @sbl-matter You agreed that the change is needed. Can you please approve again, or add someone with the right permissions to do so, as you're assigned as reviewer and the PR can finally be merged. We would really appreciate it. Thanks in advance.

@sbl-matter
Copy link

sbl-matter commented Mar 29, 2022

Sorry @shurik-io , i reviewed, as I think you could do too, thinking that would speed up the process but apparently only somebody with write access can do anything.

Copy link

@sbl-matter sbl-matter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This merge would me much appreciated

@shurik-io
Copy link

but apparently only somebody with write access can do anything.

@sbl-matter can you ping someone with write access to have a look and approve it, as I don't know who could do it. Thanks in advance.

@sbl-matter
Copy link

@lonegunmanb
Hi, any chance you'd look into this ?

@eyenx
Copy link
Contributor

eyenx commented Apr 6, 2022

Can this be merged? :)

@RobMokkink
Copy link

Can this be picked up as well? Thanks

@wrkode
Copy link

wrkode commented Apr 6, 2022

Yes, please waiting for this to be merged

@@ -289,3 +289,9 @@ variable "enable_host_encryption" {
type = bool
default = false
}

variable "node_resource_group" {
description = "The auto-generated Resource Group which contains the resources for this Managed Kubernetes Cluster."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the description " (Optional) The name of the Resource Group where the Kubernetes Nodes should exist. Changing this forces a new resource to be created." be better?

@lonegunmanb lonegunmanb merged commit bba3ecb into Azure:master Apr 13, 2022
@andypanix
Copy link

@lonegunmanb thanks for merging the PR, it would be great now if a new release would be available, could you publish the 4.15.0 release?

Many thanks!

@lonegunmanb
Copy link
Member

Hi @andypanix , this feature has been released as 4.15.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include node_resource_group as variable
10 participants