-
Notifications
You must be signed in to change notification settings - Fork 38.9k
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
[Federation][init-10a] Make the federation API server and controller manager image names configurable. #36045
[Federation][init-10a] Make the federation API server and controller manager image names configurable. #36045
Conversation
Jenkins GCI GKE smoke e2e failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
Jenkins GCE e2e failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
Jenkins unit/integration failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
Jenkins Kubemark GCE e2e failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
Jenkins GCE etcd3 e2e failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
Jenkins GKE smoke e2e failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
Jenkins GCE Node e2e failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
Jenkins GCI GCE e2e failed for commit 573a454989950423804700313ec938b0fc7310db. Full PR test history. The magic incantation to run this job again is |
573a454
to
ea75e0f
Compare
Jenkins verification failed for commit ea75e0f. Full PR test history. The magic incantation to run this job again is |
Thanks @madhusudancs. Only nits, then LGTM. Feel free to apply the label once addressed to your satisfaction. |
@quinton-hoole I don't see the comments (nits). It is not published? |
…manager image names configurable. This enables testing non-release images.
ea75e0f
to
8d8eca5
Compare
util.AddSubcommandFlags(cmd) | ||
cmd.Flags().String("dns-zone-name", "", "DNS suffix for this federation. Federated Service DNS names are published with this suffix.") | ||
cmd.Flags().String("image", defaultImage, "Image to use for federation API server and controller manager binaries.") |
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.
nit: Add an example to illustrate a valid value.
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.
@quinton-hoole defaultImage
is printed in the help text. I think that illustrates a valid value. If I were to add an example, I would just repeat that.
@@ -109,8 +110,11 @@ func NewCmdInit(cmdOut io.Writer, config util.AdminConfig) *cobra.Command { | |||
}, | |||
} | |||
|
|||
defaultImage := fmt.Sprintf("%s:%s", hyperkubeImageName, version.Get()) |
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.
nit: So we default to the explicit version of this build/release? Should we not default to the latest version rather?
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.
@quinton-hoole Yeah, that's the most ideal default behavior. But there is no good/stable way to automatically detect the latest version right now. There is no API or anything as such to determine the latest version on the client machines and no technique is future proof. Given that, I think this is a reasonable default.
@madhusudancs Yes, sorry, I forgot to press the "publish" button :-( |
This enables testing non-release images.
Please review only the last commit here. This is based on PR #35959 which will be reviewed independently.
Design Doc: PR #34484
cc @kubernetes/sig-cluster-federation @nikhiljindal
This change is![Reviewable](https://camo.githubusercontent.com/2d899f4291d07d3cd2fa4aaae1e3b243f164c23fce87d30a589ace0d496a444c/68747470733a2f2f72657669657761626c652e6b756265726e657465732e696f2f7265766965775f627574746f6e2e737667)