Promote init containers to GA #38382

hodovska · 2016-12-08T12:43:34Z

This is proposed for 1.6
PR moves beta proved concept for init containers to stable. Specification of init containers can be now stated under initContainers field in PodSpec/PodTemplateSpec. Specifying init-containers in annotation is still possible, but will be removed in future version.

Init containers have graduated to GA and now appear as a field.  The beta annotation value will still be respected and overrides the field value.

k8s-reviewable · 2016-12-08T12:43:40Z

This change is

smarterclayton · 2016-12-08T18:25:57Z

pkg/api/v1/conversion.go

-		return err
-	}
-
-	if old := out.Annotations; old != nil {


Code that uses init containers (which is mostly using versioned clients now) still needs to respect the beta annotation. So this code needs to move into a function that you can use to, if the annotation is set, override the value that is provided with the object. For backwards compatibility with APIs "beta" annotations take precedence over fields.

Is it true that this file contains only functions in format

func Convert_v1_Field_To_api_Field(in *Field, out *api.Field, s conversion.Scope) error

where type of Field is defined in types.go?

Mentioned function would need access to Annotations and InitContainers field, that means pointer to Pod. Is there a file where we put this kind of functions? Is it more acceptable not to modify conversions for init containers promotion?

@jwforres pointing out to this question

derekwaynecarr · 2016-12-12T15:18:12Z

@hodovska -- can you split the generated code into a separate commit to allow for easier review?

hodovska · 2016-12-13T12:54:14Z

Does also proposal file have to be updated? (https://github.com/kubernetes/kubernetes/blob/master/docs/proposals/container-init.md) Speaking of statements such as "While pod containers are in alpha state..."

jwforres · 2016-12-13T15:43:12Z

pkg/api/v1/types_swagger_doc_generated.go

@@ -1274,22 +1274,24 @@ var map_PodSpec = map[string]string{
 	"imagePullSecrets":              "ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. If specified, these secrets will be passed to individual puller implementations for them to use. For example, in the case of docker, only DockerConfig type secrets are honored. More info: http://kubernetes.io/docs/user-guide/images#specifying-imagepullsecrets-on-a-pod",
 	"hostname":                      "Specifies the hostname of the Pod If not specified, the pod's hostname will be set to a system-defined value.",
 	"subdomain":                     "If specified, the fully qualified Pod hostname will be \"<hostname>.<subdomain>.<pod namespace>.svc.<cluster domain>\". If not specified, the pod will not have a domainname at all.",
+	"initContainers":                "List of initialization containers belonging to the pod. Init containers are executed in order prior to containers being started. If any init container fails, the pod is considered to have failed and is handled according to its restartPolicy. The name for an init container or normal container must be unique among all containers. Init containers may not have Lifecycle actions, Readiness probes, or Liveness probes. The resourceRequirements of an init container are taken into account during scheduling by finding the highest request/limit for each resource type, and then using the max of of that value or the sum of the normal containers. Limits are applied to init containers in a similar fashion. Init containers cannot currently be added or removed. Cannot be updated. More info: http://kubernetes.io/docs/user-guide/containers",


noticed a typo, two of's "the max of of that"

derekwaynecarr · 2016-12-15T16:08:49Z

The proposals have moved to kubernetes/community repo, so an update will be needed there.

A few additional asks for this PR:

i think we need to error if there is a conflict between the value stored as an annotation versus the non-annotated form. maybe @smarterclayton has some thoughts there, but i think we should do something to handle conflict resolution in this pr.
the existing e2e tests all verify the function works when its set via annotation. i think we need to have test coverage that verifies the function works even if the annotation is not set. see: https://github.com/kubernetes/kubernetes/blob/master/test/e2e/common/init_container.go for the tests in question.

hodovska · 2017-01-02T14:27:50Z

Conversion:

v1 => internal api
possible ways to specify initContainers are: metadata.annotations(PodInitContainerStatusesBetaAnnotationKey, PodInitContainerStatusesAnnotationKey) or field spec.initContainers, but when both are filled error is shown and conversion stops. After conversion metadata.annotations values with keys mentioned above are deleted
internal api => v1
value in spec.initContainers is moved to metadata.annotation fields. It has to be removed from spec.initContainers, so following conversion to internal api will be with no errors. (- this causes huge issues, conversion shouldn't be modified this way)

We might use different, where conversion stays as is and possible conflict (annotations vs. spec.initContainers) is resolved in validation. For this I will need more information of how to run v1 validation (before conversion to internal api).

smarterclayton · 2017-01-02T22:27:37Z

We generally do not do conflict resolution, we simply treat the annotation as a complete override.

…

On Mon, Jan 2, 2017 at 10:28 AM, k8s-ci-robot ***@***.***> wrote: Jenkins GCE etcd3 e2e *failed* <https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/38382/pull-kubernetes-e2e-gce-etcd3/10913/> for commit 38d73b4 <38d73b4>. Full PR test history <http://pr-test.k8s.io/38382>. The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue <https://github.com/kubernetes/kubernetes/issues?q=is:issue+label:kind/flake+is:open> when you hit one in your PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#38382 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_pyY0h3K5iDXzUB4nfUuegwQBZ_noks5rOReBgaJpZM4LHxx_> .

smarterclayton · 2017-02-01T21:05:40Z

pkg/api/types.go

@@ -1911,6 +1909,8 @@ type PodSpec struct {
 	// If not specified, the pod will be dispatched by default scheduler.
 	// +optional
 	SchedulerName string
+	// List of initialization containers belonging to the pod.
+	InitContainers []Container


Why was this moved to the bottom?

@smarterclayton new ID of json field has to be assigned. I found this approach least impacting, since ID's have to be sorted - I didn't intend to influence other fields.

We don't sort based on id, and ids aren't added by sort. Order of fields is supposed to be human / use first, machine last. Move these back.

smarterclayton · 2017-02-01T21:05:48Z

pkg/api/v1/types.go

-	// Init containers are in alpha state and may change without notice.
-	// Cannot be updated.
-	// More info: http://kubernetes.io/docs/user-guide/containers
-	InitContainers []Container `json:"-"  patchStrategy:"merge" patchMergeKey:"name"`


Leave this where it is.

smarterclayton · 2017-02-01T21:06:03Z

pkg/api/v1/types.go

-	// startTime set.
-	// Init containers are in alpha state and may change without notice.
-	// More info: http://kubernetes.io/docs/user-guide/pod-states#container-statuses
-	InitContainerStatuses []ContainerStatus `json:"-"`


Leave this where it is.

smarterclayton · 2017-02-01T21:06:50Z

A couple of comments and rebase and regenerate, and yes, LGTM

/approve

smarterclayton · 2017-02-01T21:07:56Z

Please add the release note to the PR description:

```release-note
Init containers have graduated to GA and now appear as a field.  The beta annotation value will still be respected and overrides the field value.
```

smarterclayton · 2017-02-03T01:16:13Z

/approve

k8s-github-robot · 2017-02-03T01:17:15Z

[APPROVALNOTIFIER] This PR is APPROVED

The following people have approved this PR: hodovska, smarterclayton

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [smarterclayton]
~~docs/OWNERS~~ [smarterclayton]
~~federation/OWNERS~~ [smarterclayton]
~~pkg/OWNERS~~ [smarterclayton]
~~pkg/api/v1/OWNERS~~ [smarterclayton]
~~test/OWNERS~~ [smarterclayton]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

smarterclayton · 2017-02-03T01:17:53Z

/lgtm

k8s-github-robot · 2017-02-03T05:04:49Z

Automatic merge from submit-queue (batch tested with PRs 40864, 40666, 38382, 40874)

Automatic merge from submit-queue Put initContainers to PodSpec for some statefulset examples. **What this PR does / why we need it**: Fixed #45405 The `init container` is [graduated to GA](#38382) , so some test YAML templates needs to be updated to not use `annotations`. The following are the two places that needs update: 1. [cockroachdb](https://github.com/kubernetes/kubernetes/blob/master/examples/cockroachdb/cockroachdb-statefulset.yaml) 2. [e2e statefulset test](https://github.com/kubernetes/kubernetes/tree/master/test/e2e/testing-manifests/statefulset) **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: **Release note**: ```release-note ```

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 8, 2016

k8s-github-robot assigned bgrant0607 Dec 8, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels Dec 8, 2016

bgrant0607 assigned smarterclayton and unassigned bgrant0607 Dec 8, 2016

hodovska force-pushed the init-containers-to-ga branch from 70a219d to f0ae0cf Compare December 8, 2016 16:44

smarterclayton reviewed Dec 8, 2016

View reviewed changes

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

hodovska force-pushed the init-containers-to-ga branch from f0ae0cf to bffdb0e Compare December 13, 2016 09:31

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

hodovska force-pushed the init-containers-to-ga branch from bffdb0e to 44e964a Compare December 13, 2016 10:40

hodovska mentioned this pull request Dec 13, 2016

Move init containers to stable in v1.6 kubernetes/website#1939

Merged

jwforres reviewed Dec 13, 2016

View reviewed changes

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

hodovska force-pushed the init-containers-to-ga branch from 44e964a to 1633f9e Compare December 14, 2016 09:41

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

hodovska force-pushed the init-containers-to-ga branch from 1633f9e to e16a220 Compare December 14, 2016 11:29

ashb mentioned this pull request Dec 15, 2016

Unable to get logs/output from init container #38811

Closed

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

hodovska force-pushed the init-containers-to-ga branch from e16a220 to 38d73b4 Compare January 2, 2017 14:25

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

hodovska force-pushed the init-containers-to-ga branch from 38d73b4 to bc30f47 Compare January 3, 2017 11:46

smarterclayton reviewed Feb 1, 2017

View reviewed changes

k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 1, 2017

k8s-github-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Feb 2, 2017

derekwaynecarr mentioned this pull request Feb 2, 2017

Set defaults of init containers. #39713

Merged

dhodovsk added 3 commits February 3, 2017 00:25

Show InitContainers and InitContainerStatus in PodSpec

a856188

Modify e2e test for init containers in stable

6c0191b

Init containers in GA - generated code

adf7cf1

hodovska force-pushed the init-containers-to-ga branch from 189e700 to adf7cf1 Compare February 3, 2017 00:09

k8s-github-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 3, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2017

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

k8s-github-robot merged commit 8b795e2 into kubernetes:master Feb 3, 2017

k82cn mentioned this pull request Feb 6, 2017

Enhancement for init containers annotation. #41004

Closed

3 tasks

ericchiang mentioned this pull request Mar 28, 2017

InitContainers dont work by default when moving from unversioned client to client-go kubernetes/client-go#142

Closed

xiangpengzhao mentioned this pull request Apr 20, 2017

error validating data: found invalid field initContainers for v1.PodSpec #44715

Closed

xiangpengzhao mentioned this pull request May 5, 2017

Do not use annotation for pod affinity #45318

Closed

This was referenced May 5, 2017

Put init-containers to PodSpec #45405

Closed

Put initContainers to PodSpec for some statefulset examples. #45447

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Promote init containers to GA #38382

Promote init containers to GA #38382

hodovska commented Dec 8, 2016 •

edited

Loading

k8s-reviewable commented Dec 8, 2016

smarterclayton Dec 8, 2016

hodovska Dec 12, 2016

hodovska Dec 15, 2016

derekwaynecarr commented Dec 12, 2016

hodovska commented Dec 13, 2016

jwforres Dec 13, 2016

derekwaynecarr commented Dec 15, 2016

hodovska commented Jan 2, 2017 •

edited

Loading

smarterclayton commented Jan 2, 2017 via email

smarterclayton Feb 1, 2017

hodovska Feb 2, 2017

smarterclayton Feb 2, 2017

smarterclayton Feb 1, 2017

smarterclayton Feb 1, 2017

smarterclayton commented Feb 1, 2017

smarterclayton commented Feb 1, 2017 •

edited

Loading

smarterclayton commented Feb 3, 2017

k8s-github-robot commented Feb 3, 2017

smarterclayton commented Feb 3, 2017

k8s-github-robot commented Feb 3, 2017

Promote init containers to GA #38382

Promote init containers to GA #38382

Conversation

hodovska commented Dec 8, 2016 • edited Loading

k8s-reviewable commented Dec 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derekwaynecarr commented Dec 12, 2016

hodovska commented Dec 13, 2016

Choose a reason for hiding this comment

derekwaynecarr commented Dec 15, 2016

hodovska commented Jan 2, 2017 • edited Loading

smarterclayton commented Jan 2, 2017 via email

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

smarterclayton commented Feb 1, 2017

smarterclayton commented Feb 1, 2017 • edited Loading

smarterclayton commented Feb 3, 2017

k8s-github-robot commented Feb 3, 2017

smarterclayton commented Feb 3, 2017

k8s-github-robot commented Feb 3, 2017

hodovska commented Dec 8, 2016 •

edited

Loading

hodovska commented Jan 2, 2017 •

edited

Loading

smarterclayton commented Feb 1, 2017 •

edited

Loading