Move Deployments to ReplicaSets and switch the Deployment selector to the new LabelSelector. #19840

madhusudancs · 2016-01-20T00:30:15Z

Release note for v1.2 (Copying @bgrant0607's comment from below for better visibility)

Delete all Deployments, their RCs, and their pods, and re-create new ones using selector.matchLabels.

Way to not disrupt the running pods:

Do a non-cascading delete of the Deployment and its RCs
Upgrade Kubernetes.
Create ReplicaSets corresponding to the RCs using selector.matchLabels, which will adopt the pods
Re-create the Deployment using selector.matchLabels

End of release note

@bgrant0607 I have added the "release-note-breaking-change" label, but I am not sure if this change qualifies for a release note describing "a way to achieve or approximate previous behavior, or migration that should/could be done prior to upgrade" that you mentioned in your email. Do you think this deserves a description?

Update the Deployments' API types, defaulting code, conversions, helpers
and validation to use ReplicaSets instead of ReplicationControllers and
LabelSelector instead of map[string]string for selectors.

Also update the Deployment controller, registry, kubectl subcommands,
client listers package and e2e tests to use ReplicaSets and
LabelSelector for Deployments.

Only the last two commits in this PR need reviews.

k8s-github-robot · 2016-01-20T00:36:58Z

Labelling this PR as size/XS

k8s-bot · 2016-01-20T01:20:20Z

GCE e2e test build/test passed for commit 9b31346b2f8ddff923011f67b113e8cf994207f1.

k8s-bot · 2016-01-20T02:42:11Z

GCE e2e test build/test passed for commit 81dd187f72b52816f380fd0ef3d775b417711162.

k8s-bot · 2016-01-20T08:43:26Z

GCE e2e test build/test passed for commit 7f7eb2187467126eb9eb08afcdc059c90a7cf61c.

bgrant0607 · 2016-01-22T00:42:41Z

Yes, we should describe how to migrate:

Simple way:

Delete all Deployments, their RCs, and their pods, and re-create new ones using selector.matchLabels.

Way to not disrupt the running pods:

Do a non-cascading delete of the Deployment and its RCs
Upgrade Kubernetes.
Create ReplicaSets corresponding to the RCs using selector.matchLabels, which will adopt the pods
Re-create the Deployment using selector.matchLabels

bgrant0607 · 2016-01-22T00:43:39Z

Could one of @janetkuo, @nikhiljindal, @Kargakis, or @ironcladlou please review this?

0xmichalis · 2016-01-22T10:34:42Z

This PR is insanely huge. I would propose breaking it down to smaller self-contained PRs (1. addition of Replica Sets, 2. Move Deployments to use Replica Sets, 3. Change Deployment label selector)

0xmichalis · 2016-01-22T12:28:52Z

Ah I now noticed that only the two last commits need reviewing here. Sorry for the noise

bgrant0607 · 2016-01-23T00:37:26Z

@Kargakis Yes, all the ReplicaSet PRs seemed to be blocked on the first PR in the submit queue.

madhusudancs · 2016-01-23T00:48:42Z

This PR is insanely huge. I would propose breaking it down to smaller self-contained PRs (1. addition of Replica Sets, 2. Move Deployments to use Replica Sets, 3. Change Deployment label selector)

Also just to clarify, it doesn't make sense to move deployments to replica sets and then switch to label selector in two separate PRs while having the build and all the tests pass in both the PRs.

bgrant0607 · 2016-01-23T01:33:49Z

LGTM once comments from previous PRs are addressed.

Will wait for previous PRs to merge before applying LGTM label. This will need to be rebased, regenerate code, etc., anyway.

k8s-bot · 2016-01-26T04:03:43Z

GCE e2e build/test failed for commit 2564f34c8a57386e5774af86c6a9acd472026be1.

k8s-github-robot · 2016-02-06T03:28:00Z

Labelling this PR as size/XXL

k8s-bot · 2016-02-09T05:56:06Z

GCE e2e test build/test passed for commit b929f1d16b58d2297d81335da8bad8117b2f7c78.

madhusudancs · 2016-02-09T07:02:31Z

@janetkuo PTAL.

Ran the e2e tests manually again with --test_args='--ginkgo.focus=Deployment.*' and they all passed.

janetkuo · 2016-02-09T18:56:47Z

pkg/kubectl/cmd/expose.go

@@ -149,9 +149,9 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str
 	// For objects that need a pod selector, derive it from the exposed object in case a user
 	// didn't explicitly specify one via --selector
 	if s, found := params["selector"]; found && kubectl.IsZero(s) {
-		s, err := f.PodSelectorForObject(inputObject)


Will expose support replicasets? If so, we need to add replicasets to CanBeExposed in factory.go. And modify the description of kubectl expose in this file too. This can be done in a follow up PR.

@janetkuo yes, added the support in this PR - #20928. PTAL.

k8s-bot · 2016-02-09T23:20:56Z

GCE e2e test build/test passed for commit 7f7dda7.

janetkuo · 2016-02-09T23:45:58Z

@madhusudancs LGTM other than a minor suggestion. Feel free to add the tag.

madhusudancs · 2016-02-09T23:50:58Z

@janetkuo Thanks! Addressed the comment.

Adding LGTM.

k8s-bot · 2016-02-10T00:28:26Z

GCE e2e test build/test passed for commit e7a9f30.

k8s-github-robot · 2016-02-10T00:58:17Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-02-10T01:33:48Z

GCE e2e test build/test passed for commit e7a9f30.

k8s-github-robot · 2016-02-10T02:30:34Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-02-10T02:57:32Z

GCE e2e test build/test passed for commit e7a9f30.

k8s-github-robot · 2016-02-10T02:57:42Z

Automatic merge from submit-queue

Auto commit by PR queue bot

0xmichalis · 2016-02-10T09:38:06Z

pkg/kubectl/cmd/expose.go

@@ -149,9 +149,9 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str
 	// For objects that need a pod selector, derive it from the exposed object in case a user
 	// didn't explicitly specify one via --selector
 	if s, found := params["selector"]; found && kubectl.IsZero(s) {
-		s, err := f.PodSelectorForObject(inputObject)
+		s, err := f.MapBasedSelectorForObject(inputObject)


What's the difference between MapBasedSelectorForObject and PodSelectorForObject? Do we need both?

Just saw the discussion in https://github.com/kubernetes/kubernetes/pull/19840/files#r52379928

@madhusudancs I think you can safely collapse both methods into one as expose is the only place they are used.

@Kargakis Since this is in pkg/kubectl/... and an exported function, we weren't sure who else outside Kubernetes was using this, particularly OpenStack. So we left the existing function as is. If you are certain that OpenStack doesn't use the factory, we can probably eliminate one of the two functions.

cc @bgrant0607

We are using the old method but the signatures are the same and we can repurpose factory methods however we like. That being said, let's keep it around and once we rebase and make use of the new method then I'll submit a fix to remove the old one.

Sure, thanks. We did not want to surprise.

googlebot added the cla: yes label Jan 20, 2016

madhusudancs assigned bgrant0607 Jan 20, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 20, 2016

madhusudancs force-pushed the replicaset-deployment branch from 9b31346 to 81dd187 Compare January 20, 2016 00:50

madhusudancs force-pushed the replicaset-deployment branch from 81dd187 to 7f7eb21 Compare January 20, 2016 08:07

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2016

madhusudancs added the release-note-breaking-change label Jan 21, 2016

bgrant0607 added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 22, 2016

bgrant0607 assigned 0xmichalis and unassigned bgrant0607 Jan 23, 2016

0xmichalis mentioned this pull request Jan 25, 2016

Link all the ReplicaSet controller boilerplate together. #19806

Merged

madhusudancs force-pushed the replicaset-deployment branch from 7f7eb21 to 2564f34 Compare January 26, 2016 02:25

k8s-github-robot removed the kind/new-api label Jan 26, 2016

madhusudancs force-pushed the replicaset-deployment branch from 2564f34 to a2247fe Compare February 6, 2016 03:21

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 6, 2016

janetkuo reviewed Feb 9, 2016
View reviewed changes

madhusudancs force-pushed the replicaset-deployment branch from b929f1d to 7f7dda7 Compare February 9, 2016 22:38

janetkuo added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Feb 9, 2016

janetkuo added this to the v1.2 milestone Feb 9, 2016

Address review comments.

e7a9f30

madhusudancs force-pushed the replicaset-deployment branch from 7f7dda7 to e7a9f30 Compare February 9, 2016 23:50

janetkuo mentioned this pull request Feb 9, 2016

Cascading delete deployment #18077

Merged

madhusudancs added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 9, 2016

k8s-github-robot pushed a commit that referenced this pull request Feb 10, 2016

Merge pull request #19840 from madhusudancs/replicaset-deployment

41a98b4

Auto commit by PR queue bot

k8s-github-robot merged commit 41a98b4 into kubernetes:master Feb 10, 2016

0xmichalis reviewed Feb 10, 2016
View reviewed changes

piosz mentioned this pull request Mar 4, 2016

Make sure HPA works for Deployments and enable appriopriate e2e tests #22505

Closed

david-mcmahon added the release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. label Apr 8, 2016

redbaron mentioned this pull request Jan 4, 2018

service.spec.selector is not io.k8s.apimachinery.pkg.apis.meta.v1.LabelSelector #57848

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Deployments to ReplicaSets and switch the Deployment selector to the new LabelSelector. #19840

Move Deployments to ReplicaSets and switch the Deployment selector to the new LabelSelector. #19840

madhusudancs commented Jan 20, 2016

k8s-github-robot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

bgrant0607 commented Jan 22, 2016

bgrant0607 commented Jan 22, 2016

0xmichalis commented Jan 22, 2016

0xmichalis commented Jan 22, 2016

bgrant0607 commented Jan 23, 2016

madhusudancs commented Jan 23, 2016

bgrant0607 commented Jan 23, 2016

k8s-bot commented Jan 26, 2016

k8s-github-robot commented Feb 6, 2016

k8s-bot commented Feb 9, 2016

madhusudancs commented Feb 9, 2016

janetkuo Feb 9, 2016

madhusudancs Feb 9, 2016

k8s-bot commented Feb 9, 2016

janetkuo commented Feb 9, 2016

madhusudancs commented Feb 9, 2016

k8s-bot commented Feb 10, 2016

k8s-github-robot commented Feb 10, 2016

k8s-bot commented Feb 10, 2016

k8s-github-robot commented Feb 10, 2016

k8s-bot commented Feb 10, 2016

k8s-github-robot commented Feb 10, 2016

0xmichalis Feb 10, 2016

0xmichalis Feb 10, 2016

madhusudancs Feb 10, 2016

0xmichalis Feb 10, 2016

madhusudancs Feb 10, 2016

Move Deployments to ReplicaSets and switch the Deployment selector to the new LabelSelector. #19840

Move Deployments to ReplicaSets and switch the Deployment selector to the new LabelSelector. #19840

Conversation

madhusudancs commented Jan 20, 2016

k8s-github-robot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

bgrant0607 commented Jan 22, 2016

bgrant0607 commented Jan 22, 2016

0xmichalis commented Jan 22, 2016

0xmichalis commented Jan 22, 2016

bgrant0607 commented Jan 23, 2016

madhusudancs commented Jan 23, 2016

bgrant0607 commented Jan 23, 2016

k8s-bot commented Jan 26, 2016

k8s-github-robot commented Feb 6, 2016

k8s-bot commented Feb 9, 2016

madhusudancs commented Feb 9, 2016

janetkuo Feb 9, 2016

Choose a reason for hiding this comment

madhusudancs Feb 9, 2016

Choose a reason for hiding this comment

k8s-bot commented Feb 9, 2016

janetkuo commented Feb 9, 2016

madhusudancs commented Feb 9, 2016

k8s-bot commented Feb 10, 2016

k8s-github-robot commented Feb 10, 2016

k8s-bot commented Feb 10, 2016

k8s-github-robot commented Feb 10, 2016

k8s-bot commented Feb 10, 2016

k8s-github-robot commented Feb 10, 2016

0xmichalis Feb 10, 2016

Choose a reason for hiding this comment

0xmichalis Feb 10, 2016

Choose a reason for hiding this comment

madhusudancs Feb 10, 2016

Choose a reason for hiding this comment

0xmichalis Feb 10, 2016

Choose a reason for hiding this comment

madhusudancs Feb 10, 2016

Choose a reason for hiding this comment