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

Make default metadata, labels and tags optional #282

Conversation

kevang
Copy link

@kevang kevang commented Oct 16, 2019

SUMMARY
A number of default tags, metadata and labels are added to the nodepools by the different modules. This is fine when creating a new nodepool, but can be problematic when importing an existing nodepool that does not have those, as a new plan will lead to nodepool recreation, for example:

~ labels            = { # forces replacement
     + "cluster_name" = "cluster-somename-9965"
     + "node_pool"    = "basic-pool"
}

PROPOSED CHANGE
The proposed change makes it possible to use the modules with existing gke clusters/nodepools. It's not introducing any breaking changes and was tested with the beta private cluster. I'm happy to update the other modules, if you think the change makes sense.

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

@kevang Please update this in the configuration in autogen/ then use make generate.

@kevang
Copy link
Author

kevang commented Oct 16, 2019

Thanks @morgante, I updated it (not sure if I needed to reformat the generated files, cause is seems that a couple of lines have changed and the build is failing)

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Looks like tests/linting is failing. Can you make sure they pass locally?

@kevang
Copy link
Author

kevang commented Oct 16, 2019

I reformatted and the linting tests all pass, yet I still get Error: The following tests have failed: check_documentation. I tried make docker_generate_docs but it doesn't seem to make any changes, not sure if there's anything else I'm missing

@morgante
Copy link
Contributor

@kevang Looks like the linters are passing now, just waiting for integration tests to pass before merging.

@kevang
Copy link
Author

kevang commented Oct 18, 2019

@morgante do you think that it's ok to merge now that the checks have passed? 🙂

@morgante morgante merged commit c227c65 into terraform-google-modules:master Oct 21, 2019
@aaron-lane aaron-lane changed the title Make default metadata, labels and tags optional in beta private cluster Make default metadata, labels and tags optional Oct 24, 2019
aaron-lane added a commit that referenced this pull request Oct 24, 2019
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.

None yet

2 participants