-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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
#4544 - Fix inadvertent whitespace in v1beta3 enablement #4735
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. 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 the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
//avoid inadvertent white spaces in the apiserver config file | ||
//i.e. KUBE_API_ARGS="--runtime_config=api/v1beta3 " | ||
v1beta3 := false | ||
for k, _ := range s.RuntimeConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, but this isn't the right "fix" for this problem. It needs to be addressed in pkg/util/configuration_map.go, by banning whitespace in key names.
@bgrant0607 - yeah was looking at that originally and for some reason I could not strip any of the white space in the key names, let me try again and see if I am more successful, maybe I was stripping the value and not the key |
@bgrant0607 - so when I originally was messing with this on Friday, I wasn't trimming the else, I think this will solve the problem for efficiently. Not sure how to update a Pull Request, is that even possible or do I need to create a new pull request? func (m _ConfigurationMap) Set(value string) error { |
Pushing to the same branch updates the PR automatically. Will take a look, thanks. |
@@ -205,7 +205,8 @@ func (s *APIServer) Run(_ []string) error { | |||
glog.Fatalf("Failure to start kubelet client: %v", err) | |||
} | |||
|
|||
_, v1beta3 := s.RuntimeConfig["api/v1beta3"] | |||
_, v1beta3 := s.RuntimeConfig["api/v1beta3"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You introduced a space here by accident.
LGTM. |
Please fix the space and squash your commits. |
@bgrant0607 - so I think I finally figured out how to reset the other failed commits and "squash"into a single file commit (which I intended all along). Thanks for your patience and let me know if I still have something that I need to do |
Thanks. Have you completed the CLA, or do you have a corporate CLA? If the latter, what company are you with? |
@bgrant0607 - I signed the individual CLA, not sure about the corporate CLA, but I'm with Redhat. |
CLAs look good, thanks! |
#4544 - Fix inadvertent whitespace in v1beta3 enablement
This was an attempt to fix an issue that I discovered when enabling v1beta3 in the apiserver config. I accidentally had an extra trailing whitespace in the KUBE_API_ARGS and v1beta3 was never recognized. I tried to find a way to trim this value when it's loaded into the api server but was not successful. So the proposed solution will avoid inadvertent white spaces in the --runtime_config= string.