Trying to update ReplicaSet Docs as per issue 12081 #12409

juandiegopalomino · 2019-01-28T17:50:20Z

To start with, I tried to cleanup the docs to adhere to the style guide https://kubernetes.io/docs/contribute/style/style-guide/. I then added some description of the ReplicaSet-Pod link via the owner reference field and behavior by it. I also made a clarification on the ReplicaSet demo where it was being redundant for the sake of demonstrating the different forms of usage. I consider this draft incomplete as I still lack knowledge of how the pod labels affect the behavior.

Preview: https://deploy-preview-12409--kubernetes-io-master-staging.netlify.com/docs/concepts/workloads/controllers/replicaset/

cc @steveperry-53 @chenopis

To start with, I tried to cleanup the docs to adhere to the style guide https://kubernetes.io/docs/contribute/style/style-guide/. I then added some description of the ReplicaSet-Pod link via the owner reference field and behavior by it. I also made a clarification on the ReplicaSet demo where it was being redundant for the sake of demonstrating the different forms of usage. I consider this draft incomplete as I still lack knowledge of how the pod labels affect the behavior.

steveperry-53 · 2019-01-28T18:09:14Z

@juandiegopalomino Thanks for getting this going. I'll review and comment today.

steveperry-53 · 2019-01-28T18:09:47Z

@juandiegopalomino @chenopis

Here's an experiment I would do if I were investigating ReplicaSets. Create a ReplicaSet that has 3 pods that run gcr.io/google-samples/hello-app:2.0. Verify that each Pod has an ownerReference equal to the ReplicaSet. Next create an individual Pod that has the same labels, but runs a different image, say gcr.io/google-samples/node-hello:1.0. Verify that all 4 pods continue to run. Now delete one of the Pods from the ReplicaSet. Now you have 3 Pods, all with labels that match the selector in the ReplicaSet. What happens? Does the ReplicaSet controller create a new Pod? Or does the ReplicaSet controller recruit the singleton Pod by adding an ownerReference to the singleton Pod? If the answer is to recruit the singleton Pod, then we have a weird situation where the Pods in a ReplicaSet are not all running the same image.

OK, so I tried that experiment, and it didn't go the way I expected. Because the singleton Pod had no ownerReference, the ReplicaSet adopted the singleton Pod. At that point, the ReplicaSet had too many Pods, so it killed the singleton Pod.

steveperry-53 · 2019-01-28T18:13:48Z

content/en/docs/concepts/workloads/controllers/replicaset.md

@@ -10,17 +10,27 @@ weight: 10

 {{% capture overview %}}

-ReplicaSet is the next-generation Replication Controller. The only difference
+ReplicaSet is a type of Replication Controller. The only difference


ReplicaSet is a replacement for the old ReplicationController. So it's not really a type of replication controller. We can probably remove all mention of ReplicationController from this topic, except for one statement that mentions ReplicationController as a thing of the past.

steveperry-53 · 2019-01-28T18:17:06Z

content/en/docs/concepts/workloads/controllers/replicaset.md

+## How a ReplicaSet Works
+
+Just like a ReplicationController, a ReplicaSet's purpose is to maintain a stable set of replica Pods available at any
+given time. It does so by creating and deleting a pods as needed to reach the desired number. The link a ReplicaSet has


Our style is to capitalize the names of Kubernetes objects. So Pod should be capitalized throughout the topic.

steveperry-53 · 2019-01-28T18:30:40Z

content/en/docs/concepts/workloads/controllers/replicaset.md

-While ReplicaSets can be used independently, today it's mainly used by
-[Deployments](/docs/concepts/workloads/controllers/deployment/) as a mechanism to orchestrate pod
-creation, deletion and updates. When you use Deployments you don't have to worry
+While ReplicaSets can be used independently, it could also be used by other resources. For example,


I think rather than saying ReplicaSets can be used by Deployments, we should say that creating a Deployment is the recommended way of using a ReplicaSet.

steveperry-53 · 2019-01-29T03:28:05Z

Here's an experiment that illustrates how things can go wrong.

Create a Pod that has label steve: perry, and runs gcr.io/google-samples/hello-app:2.0. Create a second Pod that has label steve: perry, and runs gcr.io/google-samples/node-hello:1.0. Now create this ReplicaSet.

apiVersion: apps/v1
kind: ReplicaSet
metadata:
  name: my-repset
  labels:
    steve: perry
spec:
  replicas: 3
  selector:
    matchLabels:
      steve: perry
  template:
    metadata:
      labels:
        steve: perry
    spec:
      containers:
      - name: my-container
        image: nginx

What happens?
Because the first two Pods don't have owner references, the newly created ReplicaSet adopts the two Pods. Next the ReplicaSet controller creates a third Pod that runs nginx. So now the ReplicaSet has three Pods, all running different images.

Now delete the ReplicaSet. All three Pods get deleted. The first two Pods never signed up to be part of the ReplicaSet. But they got adopted, and now die along with the ReplicaSet.

What can we learn from this pathological example?
We can learn that there really is no such thing as a desired state? At first glance, it looks like the ReplicaSet specifies a desired state of three Pods running nginx. But the ReplicaSet controller does not recognize the "running nginx" as part of some desired state. The ReplicaSet controller takes action (or not) based only on Pod labels and Pod owner references. If there are exactly three Pods that meet the following conditions, the ReplicaSet controller takes no further action:

The Pod has label steve: perry.
The Pod has an ownerReference with name: my-repset and controller: true.

ownerReferences:
    - apiVersion: apps/v1
      blockOwnerDeletion: true
      controller: true
      kind: ReplicaSet
      name: my-repset

juandiegopalomino · 2019-01-29T03:42:16Z

@steveperry-53 attempting the experiment you recommended, I found that the replicaset only brings up a lone additional pod, and set the owner reference of the first two to itself. This is not desired as the replicaset is now maintaining pods which do not behave as specified in the template. I'm still reading over the email thread with Dominik for the initial experiment and the unexpected conclusion.

juandiegopalomino · 2019-01-29T03:59:33Z

So based on the 2 experiments it appears that the replicaset bases its behaviors on pods that it currently has, and those that it can immediately inherit. When brought up, it will immediately inherit those it can based on its label selectors prior to creating new pods, and if its is first brought up and then pods it can inherit appear, it will treat them as new pods in its sets and tolerate/destroy them accordingly.
To my understanding, inheritance is based on the pod matching the label selectors of the replicaset and not having an owner reference which has controller.
Do you concur? If so, I think we should add the yamls of this experiment as new examples to warn of this behavior.

dtornow · 2019-01-29T04:30:51Z

I would not use the term "inherit". Instead, I would use the term "acquire", and the term "release". The ReplicaSet Controller acquires a Pod if the Pod's labels match the ReplicaSet's label selector and the Pod does not have an OwnerReference where .Controller = True. The ReplicaSet releases a Pod if the Pod's labels do not match the ReplicaSet's label selector and the Pod has an OwnerReference to the ReplicaSet where .Controller = True

dtornow · 2019-01-29T04:34:04Z

"This is not desired as the replicaset is now maintaining pods which do not behave as specified in the template." This is by design - although counter intuitive in the beginning.

steveperry-53 · 2019-01-29T05:13:27Z

Here's my big batch of recommendations for this topic. These are just my opinions, so use them, or not, depending on how you see fit.

Don't talk about ReplicationController. Make one statement, at the end of the topic, about ReplicationController being the old thing.

Don't talk so much about Deployment. Make one statement that says Deployment is recommended.

Make the YAML examples more concise.

Don't show both matchLabels and matchExpressions in the same YAML file.
Don't include comments in the YAML files.
Don't veer off into teaching about things like DNS and resource requests. Stick to the fundamentals that we're trying to convey in this topic.
Don't include containerPort. It's a misleading field.

If you do show commands and output, our style is to separate the command from this output.

Enter this command to create the ReplicaSet:
kubectl create -f frontend.yaml

Describe the ReplicaSet:
kubectl describe replicaset frontend

The output shows blah blah:
Name: frontend
Namespace: default
Selector: tier=frontend,tier in (frontend)
Labels: app=guestbook
tier=frontend
…

Don't show output just for the sake of it. Only show the portion of the output that illustrates the ideas you are talking about.

In a concept topic, if you find yourself showing a bunch of commands and output, consider splitting those off into a Task topic. Example Task topic

Some of the text in the current topic is not needed. For example:
"As with all other Kubernetes API objects, a ReplicaSet needs the apiVersion, kind, and metadata fields. For general information about working with manifests, see object management using kubectl. A ReplicaSet also needs a .spec section."

Change "Writing a ReplicaSet Spec" to "Writing a ReplicaSet Manifest". The manifest has a spec field, but it's confusing to call the entire manifest a spec.

The current topic has several sections that talk about the fields of a ReplicaSet manifest. It would be better to shorten all that text, and have the shortened text refer to a nearby example manifest. For example, "In the manifest you can see that every member Pod must have the app: metrics label and the department: engineering label.

As an example of text that wanders, look at the Pod Template section. The section gets side tracked by talking about overlapping labels and local container restarts.

Here's a false statement:
"The .spec.selector field is a label selector. A ReplicaSet manages all the pods with labels that match the selector. "
It is possible, though not recommended, to have Pods whose labels match the selector, but are not members of the ReplicaSet. Such Pods would have to be members of some other ReplicaSet.

Here's a statement I don't understand:
"This allows the ReplicaSet to be replaced without affecting the running pods."
I don't see how you would replace a ReplicaSet without first deleting it. And that would affect running pods.

Under Working with ReplicaSets, I think we should improve the explanation of propagationPolicy and --cascade. We might get some ideas from the Garbage Collection topic.

The current topic has a lot of passive voice that we can switch to active voice.
"Pods may be removed from a ReplicaSet by …" should be changed to, "You can remove Pods from a ReplicaSet by …".

This statement makes me uncomfortable:
"Deployment is a higher-level API object that updates its underlying ReplicaSets and their Pods in a similar fashion as kubectl rolling-update."
Can one Deployment have more than one ReplicaSet? I didn't think so. Need to investigate.

In general, the text drifts. Example:
"Deployment is a higher-level API object that updates its underlying ReplicaSets and their Pods in a similar fashion as kubectl rolling-update. Deployments are recommended if you want this rolling update functionality, because unlike kubectl rolling-update, they are declarative, server-side, and have additional features."
Just talk about Deployment. Don't drift into talking about kubectl rolling update.

dtornow · 2019-01-29T14:59:02Z

This statement makes me uncomfortable:
"Deployment is a higher-level API object that updates its underlying ReplicaSets and their Pods in a similar fashion as kubectl rolling-update."
Can one Deployment have more than one ReplicaSet? I didn't think so. Need to investigate.

AFAIK, needs verification:
1.1 A Deployment can have more than one ReplicaSet at a point in time if the DeploymentController is in the process of performing an update for the Deployment.
1.2 A Deployment can have a history of ReplicaSets
I would avoid the qualifying term "higher-level API object". There is no sound definition of higher-level. While true that a DeploymentController will create ReplicaSets for Deployments, that doesn't make a Deployment higher-level

juandiegopalomino · 2019-01-29T16:39:31Z

@dtornow how about calling it an abstraction layer?

I'm beginning to address the cr by cleaning up references to the ReplicationController and making it clear that RCs are discouraged/old. I then expanded on the behavior of ReplicaSet in the presence of pods it can acquire but are not created directly by it.

dtornow · 2019-01-29T18:40:01Z

I am generally hesitant to use the terms "abstraction" or "layer". Question: What information do you want to convey when you say "abstraction layer"?

juandiegopalomino · 2019-01-29T21:59:32Z

Note to self: double check curl commands

steveperry-53 · 2019-01-30T05:43:57Z

content/en/examples/controllers/pod-rs.yaml

@@ -0,0 +1,23 @@
+apiVersion: v1


I think this file needs to go in content/en/examples/pods. Otherwise the example verification test fails with this: examples_test.go:576: controllers/pod-rs.yaml: pod-rs does not have a test case defined

"As with all other Kubernetes API objects," etc... is present in the sibling concepts/workloads/controllers/ files, so I am hesitant to change that w/o changing the others, but I did abbreviate it. "The `.spec.template` is the only required field of the `.spec`." is false, we also need the selector Trying to address passive voice Cleaned up Writing a ReplicaSet Manifest section removed How to use a ReplicaSet section as it has redundant info from the examples and Alternatives section Expanded examples a bit Cleared up passive voice

juandiegopalomino · 2019-01-30T17:40:26Z

@steveperry-53 @chenopis ok, just tried to address all the remaining points we discussed. Pls let me know if I missed one or wrote an error

netlify · 2019-01-30T17:42:58Z

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 12d901c

https://deploy-preview-12409--kubernetes-io-master-staging.netlify.com

Capitalize Pod throughout. Link is not rendering correctly. Use () instead of [] for the path. Ending with "for the creation" seems vague to me. Maybe this: "...reach the desired number. When a ReplicaSet needs to create new Pods, it uses its Pod template." Suggestion: "is via the Pod's metadata.ownerReferences field." That way the reader won't jump to the incorrect conclusion that we're talking about the ReplicaSet's metadata.ownerReferences field. with fields, including a selector that and plans accordingly Our style for headings is sentence case. So this heading would be "How a ReplicaSet works". Several headings in this topic need to be converted to sentence case. cleaned up frontend.yaml example added example checking the Pod's owner reference being set to it's parent ReplicaSet

juandiegopalomino · 2019-01-31T17:36:39Z

@steveperry-53 my pleasure. I believe my latest commit covers your new comments. As always, please let me know of any flaws.

Suggestion: In the ReplicaSet, .spec.template.metadata.labels must match spec.selector, or ...

steveperry-53 · 2019-01-31T17:53:33Z

content/en/docs/concepts/workloads/controllers/replicaset.md

-imperative whereas Deployments are declarative, so we recommend using Deployments
-through the [`rollout`](/docs/reference/generated/kubectl/kubectl-commands#rollout) command.
+The link a ReplicaSet has to its Pods is via the Pods' [metadata.ownerReferences](/docs/concepts/workloads/controllers/garbage-collection/#owners-and-dependents)
+field, which specifies what resource the current object is owned by. All _Pods_ acquired by a ReplicaSet have their owning


No italics for Pods. So change Pods to Pods.

steveperry-53 · 2019-01-31T17:54:04Z

content/en/docs/concepts/workloads/controllers/replicaset.md

+kubectl get Pods
+```
+
+You should see Pod information similar to


Colon after "similar to"

steveperry-53 · 2019-01-31T17:55:09Z

content/en/docs/concepts/workloads/controllers/replicaset.md

+kubectl get Pods
+```
+
+The output shows that the new Pods are either already terminated, or in the process of being terminated


Colon after "being terminated"

steveperry-53 · 2019-01-31T17:55:36Z

content/en/docs/concepts/workloads/controllers/replicaset.md

+kubectl create -f http://k8s.io/examples/controllers/frontend.yaml
+```
+
+We shall see that the ReplicaSet has acquired the Pods and has only created new ones according to its spec until the


Use "You" instead of "We".

steveperry-53 · 2019-01-31T17:56:40Z

content/en/docs/concepts/workloads/controllers/replicaset.md

@@ -157,7 +252,8 @@ If you do not specify `.spec.replicas`, then it defaults to 1.

 To delete a ReplicaSet and all of its Pods, use [`kubectl delete`](/docs/reference/generated/kubectl/kubectl-commands#delete). The [Garbage collector](/docs/concepts/workloads/controllers/garbage-collection/) automatically deletes all of the dependent Pods by default.

-When using the REST API or the `client-go` library, you must set `propagationPolicy` to `Background` or `Foreground` in delete option. e.g. :
+When using the REST API or the `client-go` library, you must set `propagationPolicy` to `Background` or `Foreground` in delete option.


Change "in delete option" to "in the -d option".

steveperry-53 · 2019-01-31T17:57:12Z

content/en/docs/concepts/workloads/controllers/replicaset.md


-### Pod Template
+While you can create bare Pods with no problems, it is strongly recommended to make sure that the bare Pods do not have
+labels which match the selector of one of you ReplicaSets. The reason for this is because a ReplicaSet is not limited


Change "one of you" to "one of your".

steveperry-53 · 2019-01-31T17:57:50Z

content/en/docs/concepts/workloads/controllers/replicaset.md


-For [restart policy](/docs/concepts/workloads/pods/pod-lifecycle/#restart-policy), the only allowed value for `.spec.template.spec.restartPolicy` is `Always`, which is the default.
+Suppose you create the Pods after the frontend ReplicaSet has been deployed and had set up its initial Pod replicas to


Change "and had set" to "and has set".

steveperry-53 · 2019-01-31T18:38:41Z

@juandiegopalomino, Looks good. Just a few more nit picky comments.

steveperry-53 · 2019-02-02T02:29:29Z

/approve
@chenopis, Can you lgtm when you think this is ready.

k8s-ci-robot · 2019-02-02T02:29:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: steveperry-53

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:

~~content/en/OWNERS~~ [steveperry-53]

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

chenopis · 2019-02-06T18:03:50Z

content/en/docs/concepts/workloads/controllers/replicaset.md

-in a similar fashion as `kubectl rolling-update`. Deployments are recommended if you want this rolling update functionality,
-because unlike `kubectl rolling-update`, they are declarative, server-side, and have additional features. For more information on running a stateless
-application using a Deployment, please read [Run a Stateless Application Using a Deployment](/docs/tasks/run-application/run-stateless-application-deployment/).
+[`Deployment`](/docs/concepts/workloads/controllers/deployment/) is an object which can own ReplicaSets and update


BTW, @dtornow found out that Deployments do ensure that the Pods in the resulting ReplicaSets match the image spec listed in the Deployment manifest. That's one big difference between ReplicaSets by themselves vs being used via a Deployment.

chenopis · 2019-02-06T18:07:05Z

/lgtm

dtornow · 2019-02-06T18:07:38Z

I still have to investigate the actual mechanics of this: Does the Deployment Controller actually compare the Pods' Specs or does it rely on the hash in the label

juandiegopalomino · 2019-02-12T18:06:54Z

fixes #12081

k8s-ci-robot added language/en Issues or PRs related to English language cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 28, 2019

k8s-ci-robot requested review from madhusudancs and Rajakavitha1 January 28, 2019 17:50

steveperry-53 reviewed Jan 28, 2019

View reviewed changes

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 29, 2019

steveperry-53 reviewed Jan 30, 2019

View reviewed changes

refactoring link to example yaml

2a9dee2

adding pod-rs test case

32ceb9c

steveperry-53 closed this Jan 31, 2019

steveperry-53 reopened this Jan 31, 2019

juandiegopalomino added 2 commits January 31, 2019 09:38

Previous commit broke Pod example links due to casing

f5d6396

Forgot 1 comment

a08dd12

Suggestion: In the ReplicaSet, .spec.template.metadata.labels must match spec.selector, or ...

steveperry-53 reviewed Jan 31, 2019

View reviewed changes

steveperry-53 closed this Jan 31, 2019

steveperry-53 reopened this Jan 31, 2019

Addressing grammar/syntax errors

12d901c

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 2, 2019

chenopis reviewed Feb 6, 2019

View reviewed changes

k8s-ci-robot assigned chenopis Feb 6, 2019

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

k8s-ci-robot merged commit 6224377 into kubernetes:master Feb 6, 2019

juandiegopalomino deleted the issue-12081 branch February 12, 2019 18:04

zacharysarah mentioned this pull request Feb 12, 2019

Deployment documentation incorrectly describes how ReplicaSets are named. #11763

Closed

2 tasks

juandiegopalomino mentioned this pull request May 15, 2019

REQUEST: New membership for juandiegopalomino kubernetes/org#826

Closed

6 tasks


		For [restart policy](/docs/concepts/workloads/pods/pod-lifecycle/#restart-policy), the only allowed value for `.spec.template.spec.restartPolicy` is `Always`, which is the default.
		Suppose you create the Pods after the frontend ReplicaSet has been deployed and had set up its initial Pod replicas to

Trying to update ReplicaSet Docs as per issue 12081 #12409

Trying to update ReplicaSet Docs as per issue 12081 #12409

Conversation

juandiegopalomino commented Jan 28, 2019 • edited by chenopis Loading

steveperry-53 commented Jan 28, 2019

steveperry-53 commented Jan 28, 2019 • edited Loading

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

steveperry-53 commented Jan 29, 2019 • edited Loading

juandiegopalomino commented Jan 29, 2019

juandiegopalomino commented Jan 29, 2019

dtornow commented Jan 29, 2019

dtornow commented Jan 29, 2019

steveperry-53 commented Jan 29, 2019 • edited Loading

dtornow commented Jan 29, 2019

juandiegopalomino commented Jan 29, 2019

dtornow commented Jan 29, 2019

juandiegopalomino commented Jan 29, 2019

Choose a reason for hiding this comment

juandiegopalomino commented Jan 30, 2019

netlify bot commented Jan 30, 2019 • edited Loading

juandiegopalomino commented Jan 31, 2019

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

steveperry-53 commented Jan 31, 2019

steveperry-53 commented Feb 2, 2019

k8s-ci-robot commented Feb 2, 2019

chenopis Feb 6, 2019 • edited Loading

Choose a reason for hiding this comment

chenopis commented Feb 6, 2019

dtornow commented Feb 6, 2019

juandiegopalomino commented Feb 12, 2019

juandiegopalomino commented Jan 28, 2019 •

edited by chenopis

Loading

steveperry-53 commented Jan 28, 2019 •

edited

Loading

steveperry-53 commented Jan 29, 2019 •

edited

Loading

steveperry-53 commented Jan 29, 2019 •

edited

Loading

netlify bot commented Jan 30, 2019 •

edited

Loading

chenopis Feb 6, 2019 •

edited

Loading