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.

Sign up for GitHub

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

Jump to bottom

GCE: Prefer preconfigured node tags for firewalls, if available #25148

Merged
merged 1 commit into from
May 5, 2016
Merged

GCE: Prefer preconfigured node tags for firewalls, if available #25148

merged 1 commit into from
May 5, 2016
+29 −11

Conversation

a-robinson
Copy link
Contributor

It'd be nice to also refactor the way that we use NODE_TAG on the client side in gce/util.sh and NODE_INSTANCE_PREFIX on the server side in gce/configure-vm.sh, but that can be cleaned up separately.

Note that I'm currently waiting on this to build and deploy to verify it works as intended, so it shouldn't be merged until I can confirm that's done.

@roberthbailey @thockin @fabioy

@a-robinson a-robinson added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 4, 2016
@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@a-robinson a-robinson added this to the v1.2 milestone May 4, 2016
@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2016
@thockin
Copy link
Member

thockin commented May 4, 2016

LGTM: title is relnote, maybe want to clarify a bit?

@a-robinson a-robinson changed the title Don't bother computing GCE node tags if they've been preconfigured. GCE: Prefer node tags for firewalls, if available May 4, 2016
@a-robinson a-robinson changed the title GCE: Prefer node tags for firewalls, if available GCE: Prefer preconfigured node tags for firewalls, if available May 4, 2016
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 4, 2016
@a-robinson a-robinson assigned thockin and unassigned davidopp May 4, 2016
@a-robinson a-robinson added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2016
@roberthbailey
Copy link
Contributor

lgtm (assuming it passed your manual testing)

@roberthbailey roberthbailey added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label May 4, 2016
@a-robinson
Copy link
Contributor Author

Just pushed a slight tweak - in the block of env vars in configure-vm.sh that I had set it before, it would only have gotten used on GKE, not on GCE. By separating it out more like the multizone setting it'll also be used on GCE.

With that tweak, I can confirm it works.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2016
@a-robinson
Copy link
Contributor Author

PTAL, thanks

@a-robinson
Copy link
Contributor Author

The e2e failures are due to #25153

@roberthbailey roberthbailey added lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. labels May 4, 2016
@k8s-bot
Copy link

k8s-bot commented May 4, 2016

GCE e2e build/test failed for commit 20699865a6ff994dc4506c779bc6f4f6d5050513.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

@a-robinson
Copy link
Contributor Author

@K8s e2e test this please issue: #25153

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2016
@a-robinson a-robinson added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 4, 2016
@fabioy fabioy added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2016
@k8s-bot
Copy link

k8s-bot commented May 5, 2016

GCE e2e build/test passed for commit b75fa08.

@a-robinson a-robinson merged commit c65a6a4 into kubernetes:master May 5, 2016
@a-robinson a-robinson mentioned this pull request May 5, 2016
Merged
roberthbailey added a commit that referenced this pull request May 5, 2016
@roberthbailey
Merge pull request #25182 from a-robinson/automated-cherry-pick-of-#2…
80e10f0 
…5148-upstream-release-1.2

Automated cherry pick of #25148 upstream release 1.2
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

@a-robinson a-robinson mentioned this pull request May 5, 2016
Merged
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@roberthbailey
Merge pull request kubernetes#25182 from a-robinson/automated-cherry-…
3a618b6 
…pick-of-#25148-upstream-release-1.2

Automated cherry pick of kubernetes#25148 upstream release 1.2
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
@roberthbailey
Merge pull request kubernetes#25182 from a-robinson/automated-cherry-…
96fee92 
…pick-of-#25148-upstream-release-1.2

Automated cherry pick of kubernetes#25148 upstream release 1.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thockin thockin

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.
9 participants
@a-robinson @k8s-cherrypick-bot @thockin @roberthbailey @k8s-bot @googlebot @fabioy @davidopp @k8s-github-robot