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

Add user-specified kubectl arguments to addons start script #30372

Merged
merged 1 commit into from
Aug 13, 2016
Merged

Add user-specified kubectl arguments to addons start script #30372

merged 1 commit into from
Aug 13, 2016

Conversation

bbreslauer
Copy link

@bbreslauer bbreslauer commented Aug 10, 2016 •
edited by k8s-oncall
Loading

This is a simple way, using the same environment variable paradigm used throughout these scripts, to let a user specify kubectl arguments to the addons script.

fixes #30371

This change is Reviewable

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@k8s-bot
Copy link

k8s-bot commented Aug 10, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

4 similar comments
@k8s-bot
Copy link

k8s-bot commented Aug 10, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Aug 10, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Aug 10, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Aug 10, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Aug 10, 2016
@mikedanese
Copy link
Member

LGTM, @bbreslauer please sign CLA or use your @google.com email address as the author of the commit.

@mikedanese mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2016
@mikedanese
Copy link
Member

@k8s-bot ok to test

@mikedanese mikedanese added ok-to-merge release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Aug 11, 2016
@bbreslauer bbreslauer force-pushed the addons-kubectl-opts branch from 3037748 to e82c3fb Compare August 11, 2016 01:08
@googlebot
Copy link

CLAs look good, thanks!

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2016
@mikedanese mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2016
@mikedanese
Copy link
Member

@k8s-bot ok to test, issue: #IGNORE

@k8s-bot
Copy link

k8s-bot commented Aug 11, 2016

GCE e2e build/test passed for commit e82c3fb.

@k8s-bot
Copy link

k8s-bot commented Aug 11, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@mikedanese
Copy link
Member

@k8s-bot ok to smoke e2e test, issue: #IGNORE

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Aug 13, 2016

GCE e2e build/test passed for commit e82c3fb.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 9f8625f into kubernetes:master Aug 13, 2016
@MrHohn MrHohn mentioned this pull request Oct 26, 2016
Merged
@MrHohn
Copy link
Member

MrHohn commented Oct 26, 2016 •
edited
Loading

This broke addon manager when KUBECTL_OPTS is empty.

Repro as below:

$ kubectl '' get Deployment --all-namespaces -o 'go-template={{range.items}}{{.metadata.namespace}}/{{.metadata.name}} {{end}}' -l kubernetes.io/cluster-service=true
error: name cannot be provided when a selector is specified

And because the following | sed 's/<no value>//g' command, the error would not be catched by the script.

PS: Seems like it would be kubectl '' get ... instead of kubectl get ... when scripts are nested? Not sure why it will have different behaviors.

@MrHohn MrHohn mentioned this pull request Mar 23, 2017
Closed
k8s-github-robot pushed a commit that referenced this pull request Mar 24, 2017
Merge pull request #43597 from ixdy/release-1.4-addon-manager-5.1.1
5238d1c 
Automatic merge from submit-queue

Revert #28823 and #30372 in release-1.4 branch

**What this PR does / why we need it**: these two PRs were added after the kube-addon-manager was bumped to v5.1, but v5.1 is what the Kubernetes 1.4 release uses. We should match the code and functionality so we can easily rebuild the image as needed.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #43587

**Special notes for your reviewer**: I have not yet pushed a new v5.1.1 image.

**Release note**:

```release-note
NONE
```
(No release note, since this intentionally reverts functionality which doesn't exist in 1.4.)

cc @timstclair @douglas-gibbons @bbreslauer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mikedanese mikedanese

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

addon-manager should support user-provided arguments to kubectl
7 participants
@bbreslauer @googlebot @k8s-bot @mikedanese @k8s-github-robot @MrHohn @roberthbailey