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

Extract "pkg/version".PrintAndExitIfRequested() to its own package #731

Merged
merged 1 commit into from Aug 1, 2014

Conversation

yugui
Copy link
Contributor

@yugui yugui commented Aug 1, 2014

because it causes a runtime panic if a binary which has its own implementation
of "-version" flag tries to reuse a package library which indirectly depend on
"pkg/version".

e.g. If such an user-defined binary tires to link "pkg/api" or "pkg/client",
the binary fails with a runtime panic "flag redefined: version".

because it causes a runtime panic if a binary which has its own implementation
of "-version" flag tries to reuse a package library which indirectly depend on
"pkg/version".

e.g. If such an user-defined binary tires to link "pkg/api" or "pkg/client",
the binary fails with a runtime panic "flag redefined: version".
@lavalamp
Copy link
Member

lavalamp commented Aug 1, 2014

This is a good point, and very unfortunate. Is it possible, instead, to namespace all of our flags? Like go's test package. I'm not sure if that's an improvement or not over this solution, though.

Another solution would involve providing a function in the version package to install its flag, to be called from init() in commands that want to have the version flag. Just thinking out loud here, not really sure that's an improvement, either.

@yugui
Copy link
Contributor Author

yugui commented Aug 1, 2014

Automation with init() func looks overkilling just for this function.
So I think explicit function call is clearer for now.

And for namespacing all other flag, I'm happy to do it but would you mind tentatively merging this PR because the runtime panic is blocking my team?

@smarterclayton
Copy link
Contributor

I'm not sure namespacing is an improvement - in general libraries should never define flags on load/init without an explicit request for that. I'd say an explicit package require here is equivalent to importing net/http/pprof or pkg/healthz - you're making a request by importing a package for a specific behavior.

LGTM

erictune added a commit that referenced this pull request Aug 1, 2014
Extract "pkg/version".PrintAndExitIfRequested() to its own package
@erictune erictune merged commit ee9cec8 into kubernetes:master Aug 1, 2014
vishh pushed a commit to vishh/kubernetes that referenced this pull request Apr 6, 2016
Handle system UUID retrieval for Power(ppc64) Systems
k8s-github-robot pushed a commit that referenced this pull request Apr 12, 2018
Automatic merge from submit-queue (batch tested with PRs 62455, 62465, 62427, 62416, 62411). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix kubeadm upgrade ignores HighAvailability feature gate

**What this PR does / why we need it**:
This PR fixes kubernetes/kubeadm#731

**Which issue(s) this PR fixes**:
Fixes kubernetes/kubeadm#731

**Special notes for your reviewer**:
The problem is a regression introduced by [#55952](#55952) which added a --feature-gates 
flag to `kubeadm upgrade`, to be used for updating feature gates during the upgrade process (so it is update + upgrade). 

The original implementation always override the actual feature gates with the flag value, even when the flag is not set; this PR fixes this behaviour making `kubeadm upgrade` considers the flag value only if set, otherwise actual feature gates will be preserved.

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews @kubernetes/sig-cluster-lifecycle-bugs 

**Release note**:
```release-note
Fixed #731 kubeadm upgrade ignores HighAvailability feature gate
```
seans3 pushed a commit to seans3/kubernetes that referenced this pull request Apr 10, 2019
Fix header formatting for GMSA KEP
sallyom pushed a commit to sallyom/kubernetes that referenced this pull request Jun 14, 2021
BUG 1927359: Add support for gathering metrics from CSI block-mode volumes
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

4 participants