Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic #69878

Klaven · 2018-10-16T13:53:24Z

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:

Self hosted is no longer supported in the standard workflow. The feature flags have been removed and your self hosted cluster is no longer able to upgrade via kubeadm.

Klaven · 2018-10-16T13:58:10Z

/assign @fabriziopandini

neolit123 · 2018-10-16T15:12:33Z

@Klaven
thanks for the PR! could you please use a more descriptive title instead of "Kubeadm #issue"

Fixes # Kubeadm 1072

to make that work you can do:

Fixes kubernetes/kubeadm#1072

neolit123 · 2018-10-16T15:18:35Z

The function SetKubernetesVersion that was originally found in /cmd/kubeadm/app/cmd/phases/util.go was moved to the file /cmd/kubeadm/app/cmd/util/cmdutil.go
I don't like this ether but was unsure of a better place to put it. It is kind of a "common" utility not only used by the phases. If you would like this back in it's original spot that is fine although a common utill seems like a better place for this but I could not find such a place.

agreed, the current location is not great, but a separate PR would have been better.
adding related comment in the review.

neolit123

@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

neolit123 · 2018-10-16T15:18:06Z

cmd/kubeadm/app/cmd/util/cmdutil.go

+
+// 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) {


the function should be moved to this file instead:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/util/config/common.go

I will remove this entirely from this commit and make a new PR for it's move to the above file.

neolit123 · 2018-10-16T15:20:06Z

cmd/kubeadm/app/features/features.go

 // 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},


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".

@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.

I'm fine in removing all the three feature gates in one PR

neolit123 · 2018-10-16T15:21:50Z

cmd/kubeadm/app/cmd/selfhosting.go

@@ -127,4 +123,4 @@ func getSelfhostingSubCommand() *cobra.Command {
 	options.AddKubeConfigFlag(cmd.Flags(), &kubeConfigFile)

 	return cmd
-}
+}


files should end with an empty new line.

This was corrected once verify and update where run! the PR is not longer in WIP. Thanks for the comments!

Klaven · 2018-10-16T15:29:10Z

The function SetKubernetesVersion that was originally found in /cmd/kubeadm/app/cmd/phases/util.go was moved to the file /cmd/kubeadm/app/cmd/util/cmdutil.go
I don't like this ether but was unsure of a better place to put it. It is kind of a "common" utility not only used by the phases. If you would like this back in it's original spot that is fine although a common utill seems like a better place for this but I could not find such a place.

agreed, the current location is not great, but a separate PR would have been better.
adding related comment in the review.

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!

neolit123 · 2018-10-16T15:33:44Z

@Klaven

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.
the function should go here:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/util/config/common.go

fabriziopandini

@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

fabriziopandini · 2018-10-16T16:12:19Z

cmd/kubeadm/app/cmd/phases/util.go

-
-// 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) {


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

yes, I agree. I removed it and will open another PR for this change.

fabriziopandini · 2018-10-16T16:23:59Z

cmd/kubeadm/app/cmd/selfhosting.go

 )

 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.


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

@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.

@Klaven ok, with this but please address this comment in the next PR

fabriziopandini · 2018-10-16T16:25:37Z

cmd/kubeadm/app/cmd/selfhosting.go

-		# 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 self hosting (remove phases)

@Klaven ok, with this but please address this comment in the next PR

fabriziopandini · 2018-10-16T16:36:48Z

cmd/kubeadm/app/cmd/selfhosting.go

 )

 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.


I don't have opinions about changing self-hosting to selfhosting, but IMO this refactor is out of scope of this PR

+1
@Klaven let's keep the rename out of the PR for now.

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.

@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

I'm undoing the update to the naming and will only address this with the PR for adding the command.

fabriziopandini · 2018-10-16T16:57:50Z

cmd/kubeadm/app/cmd/selfhosting/selfhosting.go

-		if err := uploadKubeConfigSecrets(client, kubeConfigDir); err != nil {
-			return err
-		}
+	// Upload the certificates and kubeconfig files from disk to the cluster as Secrets


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)

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.

@Klaven ok, with this but please address this comment in the next PR

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?

fabriziopandini · 2018-10-16T17:16:49Z

cmd/kubeadm/app/cmd/selfhosting/BUILD

@@ -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",


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)

i pretty much think this first PR should have only done the removal of deprecated F-gates.

@fabriziopandini I thought this /phases/selfhosting was going to become the backbone of the new command kubeadm selfhosting pivot is this not the case?

@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)

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?

fabriziopandini · 2018-10-16T17:23:16Z

cmd/kubeadm/app/cmd/util/cmdutil.go

@@ -18,9 +18,7 @@ package util

 import (
 	"fmt"
-


Please leave spaces between standard packages, external packages, kubernetes packages

ok, my apologies. Will update this right away.

fabriziopandini · 2018-10-16T17:24:23Z

cmd/kubeadm/app/features/features.go


 	// 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


This feature gate should go away

fabriziopandini · 2018-10-16T17:25:33Z

cmd/kubeadm/app/features/features.go

 // 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},


I'm fine in removing all the three feature gates in one PR

fabriziopandini · 2018-10-16T18:00:25Z

PS please remove from the or description all the things that that old (wip, setkubernetesversion).

fabriziopandini

@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

fabriziopandini · 2018-10-19T07:28:47Z

cmd/kubeadm/app/cmd/selfhosting/BUILD

@@ -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",


@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)

fabriziopandini · 2018-10-19T07:34:06Z

cmd/kubeadm/app/cmd/selfhosting.go

 )

 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.


@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

fabriziopandini · 2018-10-19T07:36:02Z

cmd/kubeadm/app/cmd/selfhosting.go

 )

 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.


@Klaven ok, with this but please address this comment in the next PR

fabriziopandini · 2018-10-19T07:36:53Z

cmd/kubeadm/app/cmd/selfhosting.go

-		# 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. 


@Klaven ok, with this but please address this comment in the next PR

fabriziopandini · 2018-10-19T07:37:09Z

cmd/kubeadm/app/cmd/selfhosting/selfhosting.go

-		if err := uploadKubeConfigSecrets(client, kubeConfigDir); err != nil {
-			return err
-		}
+	// Upload the certificates and kubeconfig files from disk to the cluster as Secrets


@Klaven ok, with this but please address this comment in the next PR

fabriziopandini · 2018-10-19T07:41:07Z

/ok-to-test

neolit123 · 2018-11-04T17:21:05Z

IMO this PR now is well focused and fully address the issue is tackling. If you still have doubts on this PR I'm available for a pair review to better understand you point of view

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.

fabriziopandini · 2018-11-05T15:01:23Z

/retest

Klaven · 2018-11-05T15:36:46Z

/retest
bazel build has been updated.

fabriziopandini

@Klaven well done!
/approve
I kindly ask @neolit123 for a final pass and final lgtm

neolit123

@Klaven this is looking good.
added some finalizing comments about unit tests, naming self-hosting consistently and other minor fixes.
after that LGTM!

neolit123 · 2018-11-06T12:19:40Z

cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

-		{featureFlag{"StoreCertsInSecrets": true}, true},
-		{featureFlag{"StoreCertsInSecrets": false}, true},
+		{featureFlag{"Unknown": true}, false},
+		{featureFlag{"Unknown": false}, false},
 		{featureFlag{"Foo": true}, false},


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},

neolit123 · 2018-11-06T12:21:37Z

cmd/kubeadm/app/cmd/alpha/selfhosting.go

-		# 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


please remove the empty line above this 57.

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 `) )```

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?

in the new phase commands we tend to have:

# comment example code

neolit123 · 2018-11-06T12:23:38Z

cmd/kubeadm/app/cmd/alpha/selfhosting.go

 			var err error
+
+			if !forcePivot {
+				fmt.Println(


please move "WARNING..." to start on the same line as Println()

neolit123 · 2018-11-06T12:24:02Z

cmd/kubeadm/app/cmd/alpha/selfhosting.go

+
+	cmd.Flags().BoolVarP(
+		&forcePivot, "force", "f", false,
+		"Pivot the cluster without prompting for confirmation.",


please remove the dot at the end.

This would be inconsistent with how we did force in Reset as well as the other flag messages in Reset.

whatever. i'll just delete it so you will merge it.

or at least make the flag descriptions consistent in this file.
one has dot, one doesn't.

neolit123 · 2018-11-06T12:24:35Z

cmd/kubeadm/app/cmd/upgrade/common.go

@@ -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")


"[upgrade/apply] Cannot upgrade a Self-Hosted control plane"
->
"cannot upgrade a self-hosted control plane"

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.

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.

neolit123 · 2018-11-06T12:39:27Z

cmd/kubeadm/app/phases/selfhosting/selfhosting.go

@@ -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)


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

do we want the command to be self-hosting then as well?

please keep the filename selfhosting.go but the command and println() + comment should be self-hosting.

neolit123 · 2018-11-06T16:31:45Z

/lgtm
thanks @Klaven
cc @timothysc for the final approval

timothysc

/approve
/hold

Something is weird can you please dbl-check the comment below.

timothysc · 2018-11-06T15:19:57Z

cmd/kubeadm/app/cmd/alpha/selfhosting.go

@@ -14,19 +14,22 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */

-package phases
+package alpha


100644 → 100755
You're file permissions appear to be screwed up, please ensure 644

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

Klaven · 2018-11-07T18:10:37Z

@timothysc it looks like the tests are passing. this is rebased and file permissions where updated. let me know if you want anything else!

timothysc

/lgtm
/approve

k8s-ci-robot · 2018-11-07T20:14:35Z

[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:

~~cmd/kubeadm/OWNERS~~ [fabriziopandini,timothysc]
docs/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

timothysc · 2018-11-07T20:19:39Z

/hold cancel

neolit123 · 2018-11-07T21:58:18Z

/retest

k8s-ci-robot requested review from kad and timothysc October 16, 2018 13:53

k8s-ci-robot added area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 16, 2018

k8s-ci-robot assigned fabriziopandini Oct 16, 2018

Klaven changed the title ~~WIP: Kubeadm 1072~~ WIP: Fixes kubernetes/kubeadm#1072 Cleanup of selfhosting logic Oct 16, 2018

Klaven changed the title ~~WIP: Fixes kubernetes/kubeadm#1072 Cleanup of selfhosting logic~~ WIP: Fixes https://github.com/kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic Oct 16, 2018

Klaven changed the title ~~WIP: Fixes https://github.com/kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic~~ WIP: Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic Oct 16, 2018

neolit123 reviewed Oct 16, 2018

View reviewed changes

Klaven force-pushed the kubeadm_1072 branch from da3bd4a to 3abd56c Compare October 16, 2018 17:01

Klaven changed the title ~~WIP: Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic~~ Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic Oct 16, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 16, 2018

fabriziopandini suggested changes Oct 16, 2018

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2018

fabriziopandini suggested changes Oct 19, 2018

View reviewed changes

Klaven changed the title ~~[WIP] Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic~~ Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic Nov 5, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2018

Klaven force-pushed the kubeadm_1072 branch from c4dddfc to 43f3468 Compare November 5, 2018 14:47

Klaven force-pushed the kubeadm_1072 branch from 43f3468 to 986e5c3 Compare November 5, 2018 15:22

fabriziopandini approved these changes Nov 6, 2018

View reviewed changes

neolit123 reviewed Nov 6, 2018

View reviewed changes

Klaven force-pushed the kubeadm_1072 branch from 290266d to c8fa217 Compare November 6, 2018 14:29

k8s-ci-robot assigned neolit123 Nov 6, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 6, 2018

timothysc approved these changes Nov 7, 2018

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 7, 2018

timothysc added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 7, 2018

Removed feature gates selfhosting, HA and store certs in secrets.

18dc529

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

Klaven force-pushed the kubeadm_1072 branch from c8fa217 to 18dc529 Compare November 7, 2018 16:48

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2018

timothysc approved these changes Nov 7, 2018

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 7, 2018

k8s-ci-robot merged commit e41f297 into kubernetes:master Nov 8, 2018

This was referenced Nov 8, 2018

Cleanup code for feature-gates HighAvailability, SelfHosting, CertsInSecrets kubernetes/kubeadm#1072

Closed

tracking issue for self-hosting solutions and proposals kubernetes/kubeadm#1093

Closed

Klaven mentioned this pull request Feb 7, 2019

REQUEST: New membership for @Klaven (Marek Counts) kubernetes/org#455

Closed

6 tasks

Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic #69878

Fixes /kubernetes/kubeadm/issues/1072 Cleanup of selfhosting logic #69878

Conversation

Klaven commented Oct 16, 2018 • edited

Klaven commented Oct 16, 2018

neolit123 commented Oct 16, 2018

neolit123 commented Oct 16, 2018

neolit123 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Klaven commented Oct 16, 2018

neolit123 commented Oct 16, 2018

fabriziopandini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabriziopandini Oct 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabriziopandini commented Oct 16, 2018 • edited

fabriziopandini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabriziopandini commented Oct 19, 2018

neolit123 commented Nov 4, 2018

fabriziopandini commented Nov 5, 2018

Klaven commented Nov 5, 2018

fabriziopandini left a comment

Choose a reason for hiding this comment

neolit123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neolit123 commented Nov 6, 2018 • edited

Klaven commented Oct 16, 2018 •

edited

neolit123 left a comment •

edited

fabriziopandini Oct 16, 2018 •

edited

fabriziopandini commented Oct 16, 2018 •

edited

neolit123 commented Nov 6, 2018 •

edited