-
Notifications
You must be signed in to change notification settings - Fork 39k
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
Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic #69878
Conversation
/assign @fabriziopandini |
@Klaven
to make that work you can do:
|
agreed, the current location is not great, but a separate PR would have been better. |
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.
@Klaven
please keep the SetKubernetesVersion outside of the scope of this PR.
in my opinion:
- the SetKubernetesVersion() could have been a separate PR.
- the initial PR could have been removing the feature gates and naming the PR "kubeadm: remove f-gates x,y,z"
- the second PR could have been moving the selfhosting.go and also enabling it as a root command.
/cc @fabriziopandini for further comments.
thanks
cmd/kubeadm/app/cmd/util/cmdutil.go
Outdated
|
||
// SetKubernetesVersion gets the current Kubeadm version and sets it as KubeadmVersion in the config, | ||
// unless it's already set to a value different from the default. | ||
func SetKubernetesVersion(cfg *kubeadmapiv1beta1.InitConfiguration) { |
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.
the function should be moved to this file instead:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/util/config/common.go
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.
I will remove this entirely from this commit and make a new PR for it's move to the above file.
// InitFeatureGates are the default feature gates for the init command | ||
var InitFeatureGates = FeatureList{ | ||
SelfHosting: {FeatureSpec: utilfeature.FeatureSpec{Default: false, PreRelease: utilfeature.Deprecated}, HiddenInHelpText: true, DeprecationMessage: selfHostingDeprecationMessage}, | ||
StoreCertsInSecrets: {FeatureSpec: utilfeature.FeatureSpec{Default: false, PreRelease: utilfeature.Deprecated}, HiddenInHelpText: true, DeprecationMessage: storeCertsInSecretsDeprecationMessage}, | ||
HighAvailability: {FeatureSpec: utilfeature.FeatureSpec{Default: false, PreRelease: utilfeature.Deprecated}, HiddenInHelpText: true, DeprecationMessage: highAvailabilityMessage}, |
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.
this PR is not only about self-hosting it seems.
probably should have been broken down in further steps, first being "removing the f-gates for x, y, z".
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.
@neolit123 and @fabriziopandini mostly this does just remove self-hosting feature gates and the code that calls the feature gates. The only thing on top of that is that it removed the selfhosting.go file from the phases folder as it is not a phase anymore with the feature gate removed.
I was under the impression the StoreCertsInSecrets
and HighAvailability
where both features related to selfhosting and if you removed self hosting you removed both of them. If I am wrong I am happy to correct. I too felt like the PR is way to large, but minus a few small update that will be coming later today I felt like this was a good breakup of the work.
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.
I'm fine in removing all the three feature gates in one PR
cmd/kubeadm/app/cmd/selfhosting.go
Outdated
@@ -127,4 +123,4 @@ func getSelfhostingSubCommand() *cobra.Command { | |||
options.AddKubeConfigFlag(cmd.Flags(), &kubeConfigFile) | |||
|
|||
return cmd | |||
} | |||
} |
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.
files should end with an empty new line.
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.
This was corrected once verify and update where run! the PR is not longer in WIP. Thanks for the comments!
I did it with this as the self hosting logic used it. meaning that self hosting would have to import phases. If you would like another PR. I will remove it and make a new PR with it updated. Thank you for the comment! |
best if we start with the version PR. |
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.
@Klaven
The overall direction of this PR looks fine! Great!
I have left some feedbacks; let me know when this one is ready for another pass
cmd/kubeadm/app/cmd/phases/util.go
Outdated
|
||
// SetKubernetesVersion gets the current Kubeadm version and sets it as KubeadmVersion in the config, | ||
// unless it's already set to a value different from the default. | ||
func SetKubernetesVersion(cfg *kubeadmapiv1beta1.InitConfiguration) { |
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.
This change adds a lot of noise and IMO doesn't seems strictly related to the goal of this PR.
Unless there is something that I'm missing please keep this refactor out of this PR. Eventually, if you consider this relevant, please open an issue to track this and get some feedback/consensus before executing
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.
yes, I agree. I removed it and will open another PR for this change.
cmd/kubeadm/app/cmd/selfhosting.go
Outdated
) | ||
|
||
var ( | ||
selfhostingLongDesc = normalizer.LongDesc(` | ||
Converts static Pod files for control plane components into self-hosted DaemonSets configured via the Kubernetes API. | ||
|
||
See the documentation for self-hosting limitations. | ||
See the documentation for selfhosting limitations. |
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.
Considering what discussed at sig level I think it should be stated clearly both in the command help and din the online documentation that self-hosted clusters are not supported by other kubeadm workflows e.g. updates.
Please sync with @neolit123 for the online documentation
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.
@fabriziopandini I agree. this pull request does not include the command line option. It only removes the feature gates. My next PR with the command line options would have all the documentation and warnings when pivoting your cluster. At least that was my understanding.
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.
@Klaven ok, with this but please address this comment in the next PR
cmd/kubeadm/app/cmd/selfhosting.go
Outdated
# Converts a static Pod-hosted control plane into a self-hosted one, | ||
# functionally equivalent to what generated by kubeadm init executed | ||
# with --feature-gates=SelfHosting=true. | ||
# Converts a static Pod-hosted control plane into a self-hosted one. |
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.
Kubeadm alpha self hosting (remove phases)
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.
@Klaven ok, with this but please address this comment in the next PR
cmd/kubeadm/app/cmd/selfhosting.go
Outdated
) | ||
|
||
var ( | ||
selfhostingLongDesc = normalizer.LongDesc(` | ||
Converts static Pod files for control plane components into self-hosted DaemonSets configured via the Kubernetes API. | ||
|
||
See the documentation for self-hosting limitations. | ||
See the documentation for selfhosting limitations. |
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.
I don't have opinions about changing self-hosting to selfhosting, but IMO this refactor is out of scope of this PR
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.
+1
@Klaven let's keep the rename out of the PR for now.
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.
ok. originally the command line we had talked about was going to be selfhosting pivot
and all the files are called "selfhosting" so I was under the impression that it should also be called selfhosting. I will revert.
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.
@Klaven I'm ok in fixing everything is user-facing; instead fixing the code comments IMO is not necessary, but I can leave with it
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.
I'm undoing the update to the naming and will only address this with the PR for adding the command.
if err := uploadKubeConfigSecrets(client, kubeConfigDir); err != nil { | ||
return err | ||
} | ||
// Upload the certificates and kubeconfig files from disk to the cluster as Secrets |
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.
I don't think that we want that self hosting always imply store certs in secrets, so IMO feature gate StoreCertsInSecrets should be replaced with a new flag --store-certs-in-secrets (added to the new kubeadm alpha self hosting command)
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.
agreed. but this PR does not cover the new command line feature and adding the flags it requires. As of this PR, this is dead code that cant be run. Once my next PR is in this will be a thing and should be added in as a flag then. I think adding this in this PR is scope creep of the second PR to add the command to pivot the cluster.
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.
@Klaven ok, with this but please address this comment in the next PR
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.
would you like me to put a TODO
in the code and commit it to ensure it gets updated in the next PR? or just leave it as is and link these comments when I add the flags?
@@ -29,11 +29,10 @@ go_library( | |||
"selfhosting.go", | |||
"selfhosting_volumes.go", | |||
], | |||
importpath = "k8s.io/kubernetes/cmd/kubeadm/app/phases/selfhosting", | |||
importpath = "k8s.io/kubernetes/cmd/kubeadm/app/cmd/selfhosting", |
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.
Package k8s.io/kubernetes/cmd/kubeadm/app/phases/selfhosting
should not be moved under /cmd
.
Might be you lack a bit of context here, but you can consider /cmd
as UX only (cobra commands) and /app/phases
as business logic (tl dr; I think that soon or later we should move the latest to a less ambiguous /pkg
, but this wasn't discussed yet)
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.
i pretty much think this first PR should have only done the removal of deprecated F-gates.
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.
@fabriziopandini I thought this /phases/selfhosting
was going to become the backbone of the new command kubeadm selfhosting pivot
is this not the case?
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.
@Klaven yes, /phases/selfhosting
should be used as a backbone for the new command kubeadm selfhosting pivot
, but it should remain where it is (it should not be moved under kubeadm/app/cmd
)
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.
Ok, awesome. I will update it like this. sorry about that! Are we leaving it a phase then? that seems odd as we are moving it out of being a phase. do you want me to move it somewhere else or just leave it?
cmd/kubeadm/app/cmd/util/cmdutil.go
Outdated
@@ -18,9 +18,7 @@ package util | |||
|
|||
import ( | |||
"fmt" | |||
|
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.
Please leave spaces between standard packages, external packages, kubernetes packages
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.
ok, my apologies. Will update this right away.
cmd/kubeadm/app/features/features.go
Outdated
|
||
// CoreDNS is GA in v1.11 | ||
CoreDNS = "CoreDNS" | ||
|
||
// SelfHosting is alpha in v1.8 and v1.9 - deprecated in v1.12 (TODO remove in v1.13) | ||
// SelfHosting is alpha in v1.8 and v1.9 - deprecated in v1.12 - removed from workflow in v1.13 |
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.
This feature gate should go away
// InitFeatureGates are the default feature gates for the init command | ||
var InitFeatureGates = FeatureList{ | ||
SelfHosting: {FeatureSpec: utilfeature.FeatureSpec{Default: false, PreRelease: utilfeature.Deprecated}, HiddenInHelpText: true, DeprecationMessage: selfHostingDeprecationMessage}, | ||
StoreCertsInSecrets: {FeatureSpec: utilfeature.FeatureSpec{Default: false, PreRelease: utilfeature.Deprecated}, HiddenInHelpText: true, DeprecationMessage: storeCertsInSecretsDeprecationMessage}, | ||
HighAvailability: {FeatureSpec: utilfeature.FeatureSpec{Default: false, PreRelease: utilfeature.Deprecated}, HiddenInHelpText: true, DeprecationMessage: highAvailabilityMessage}, |
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.
I'm fine in removing all the three feature gates in one PR
PS please remove from the or description all the things that that old (wip, setkubernetesversion). |
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.
@Klaven
We are nearly there with this PR
Please move back the /phases/selfhosting package to the current location and then we can quickly get this approved
@@ -29,11 +29,10 @@ go_library( | |||
"selfhosting.go", | |||
"selfhosting_volumes.go", | |||
], | |||
importpath = "k8s.io/kubernetes/cmd/kubeadm/app/phases/selfhosting", | |||
importpath = "k8s.io/kubernetes/cmd/kubeadm/app/cmd/selfhosting", |
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.
@Klaven yes, /phases/selfhosting
should be used as a backbone for the new command kubeadm selfhosting pivot
, but it should remain where it is (it should not be moved under kubeadm/app/cmd
)
cmd/kubeadm/app/cmd/selfhosting.go
Outdated
) | ||
|
||
var ( | ||
selfhostingLongDesc = normalizer.LongDesc(` | ||
Converts static Pod files for control plane components into self-hosted DaemonSets configured via the Kubernetes API. | ||
|
||
See the documentation for self-hosting limitations. | ||
See the documentation for selfhosting limitations. |
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.
@Klaven I'm ok in fixing everything is user-facing; instead fixing the code comments IMO is not necessary, but I can leave with it
cmd/kubeadm/app/cmd/selfhosting.go
Outdated
) | ||
|
||
var ( | ||
selfhostingLongDesc = normalizer.LongDesc(` | ||
Converts static Pod files for control plane components into self-hosted DaemonSets configured via the Kubernetes API. | ||
|
||
See the documentation for self-hosting limitations. | ||
See the documentation for selfhosting limitations. |
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.
@Klaven ok, with this but please address this comment in the next PR
cmd/kubeadm/app/cmd/selfhosting.go
Outdated
# Converts a static Pod-hosted control plane into a self-hosted one, | ||
# functionally equivalent to what generated by kubeadm init executed | ||
# with --feature-gates=SelfHosting=true. | ||
# Converts a static Pod-hosted control plane into a self-hosted one. |
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.
@Klaven ok, with this but please address this comment in the next PR
if err := uploadKubeConfigSecrets(client, kubeConfigDir); err != nil { | ||
return err | ||
} | ||
// Upload the certificates and kubeconfig files from disk to the cluster as Secrets |
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.
@Klaven ok, with this but please address this comment in the next PR
/ok-to-test |
SGTM if you are fine with the scope. the PR work is "over the fence" at this point, so we might as well finish it. |
/retest |
/retest |
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.
@Klaven well done!
/approve
I kindly ask @neolit123 for a final pass and final lgtm
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.
@Klaven this is looking good.
added some finalizing comments about unit tests, naming self-hosting consistently and other minor fixes.
after that LGTM!
{featureFlag{"StoreCertsInSecrets": true}, true}, | ||
{featureFlag{"StoreCertsInSecrets": false}, true}, | ||
{featureFlag{"Unknown": true}, false}, | ||
{featureFlag{"Unknown": false}, false}, | ||
{featureFlag{"Foo": true}, false}, |
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.
i think Foo is the same as Unknown here.
please remove the line with Foo.
also please add these valid feature gate tests:
{featureFlag{"CoreDNS": true}, true},
{featureFlag{"CoreDNS": false}, true},
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.
👍
# Converts a static Pod-hosted control plane into a self-hosted one, | ||
# functionally equivalent to what generated by kubeadm init executed | ||
# with --feature-gates=SelfHosting=true. | ||
# Converts a static Pod-hosted control plane into a self-hosted one. | ||
|
||
kubeadm alpha phase selfhosting convert-from-staticpods |
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.
please remove the empty line above this 57
.
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.
this would then be inconsistent with the ones around it, are you sure?
selfhostingLongDesc = normalizer.LongDesc(`
Converts static Pod files for control plane components into self-hosted DaemonSets configured via the Kubernetes API.
See the documentation for self-hosting limitations.
` + cmdutil.AlphaDisclaimer)
selfhostingExample = normalizer.Examples(`
# Converts a static Pod-hosted control plane into a self-hosted one.
kubeadm alpha phase self-hosting convert-from-staticpods
`)
)```
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.
I'll do it. It's just how it was, and it matches the other cases above it. If you do really want me to change it do you want me to change the code above it too to match?
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.
in the new phase commands we tend to have:
# comment
example code
var err error | ||
|
||
if !forcePivot { | ||
fmt.Println( |
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.
please move "WARNING..." to start on the same line as Println()
|
||
cmd.Flags().BoolVarP( | ||
&forcePivot, "force", "f", false, | ||
"Pivot the cluster without prompting for confirmation.", |
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.
please remove the dot at the end.
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.
This would be inconsistent with how we did force in Reset
as well as the other flag messages in Reset.
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.
whatever. i'll just delete it so you will merge it.
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.
or at least make the flag descriptions consistent in this file.
one has dot, one doesn't.
@@ -58,6 +58,11 @@ func enforceRequirements(flags *applyPlanFlags, dryRun bool, newK8sVersion strin | |||
return nil, errors.Wrapf(err, "couldn't create a Kubernetes client from file %q", flags.kubeConfigPath) | |||
} | |||
|
|||
// Check if the cluster is self-hosted | |||
if upgrade.IsControlPlaneSelfHosted(client) { | |||
return nil, errors.Errorf("[upgrade/apply] Cannot upgrade a Self-Hosted control plane") |
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.
"[upgrade/apply] Cannot upgrade a Self-Hosted control plane"
->
"cannot upgrade a self-hosted control plane"
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.
I am not sure why you want to remove the [upgrade/apply]
Please explain so that I understand this logic in the future. This code is run only from plan and apply in the upgrade process.
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.
we are just consistent about not having the [....] prefix in return errors.
it's also a go pattern to have errors simple, propagate them up and leave it to the caller to handle formatting.
@@ -49,26 +48,24 @@ const ( | |||
// It achieves that task this way: | |||
// 1. Load the Static Pod specification from disk (from /etc/kubernetes/manifests) | |||
// 2. Extract the PodSpec from that Static Pod specification | |||
// 3. Mutate the PodSpec to be compatible with self-hosting (add the right labels, taints, etc. so it can schedule correctly) | |||
// 3. Mutate the PodSpec to be compatible with selfhosting (add the right labels, taints, etc. so it can schedule correctly) |
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.
we are not consistent with naming of the feature.
please rename all comments and user outputs (printf, println...) to be self-hosting
as per the wikipedia definition:
https://en.wikipedia.org/wiki/Self-hosting
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.
do we want the command to be self-hosting
then as well?
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.
please keep the filename selfhosting.go but the command and println() + comment should be self-hosting.
/lgtm |
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.
/approve
/hold
Something is weird can you please dbl-check the comment below.
@@ -14,19 +14,22 @@ See the License for the specific language governing permissions and | |||
limitations under the License. | |||
*/ | |||
|
|||
package phases | |||
package alpha |
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.
100644 → 100755
You're file permissions appear to be screwed up, please ensure 644
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.
done. I have no idea how this happened. very nice catch btw. Thank you!
Added new alpha command to pivot to self hosted Removed slelfhosting upgrade ability Added warning message to self hosted pivot added certs in secrets flag to new selfhosting comand
@timothysc it looks like the tests are passing. this is rebased and file permissions where updated. let me know if you want anything else! |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: fabriziopandini, Klaven, timothysc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
/retest |
What this PR does / why we need it:
This PR removes self hosted from the standard workflow and completely removes the upgrade portion. We need this because this is what we discussed in Kubeadm office hours on Wed 3 Oct 2018. Per discussion a new command
kubeadm selfhosted pivot
has been added along with flags that support store certs in secrets in the self hosted command to replace the removed feature gate. The kubeadm ticket can be found here: kubernetes/kubeadm#1072--UPDATE--
Was instructed to not only remove selfhosting but to add the new command in one large PR, the description has been updated to reflect this.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes # Kubeadm 1072
Special notes for your reviewer:
None
Release note: