Add statefulset conditions #55268

foxish · 2017-11-07T21:02:34Z

What this PR does / why we need it: This PR adds conditions to statefulset status making it consistent with other controllers for v1. See linked issue for discussion.

/xref: #7856

Special notes for your reviewer:

Release note:

StatefulSet status now has support for conditions, making it consistent with other core controllers in v1

smarterclayton · 2017-11-07T23:02:23Z

pkg/apis/apps/types.go

+	// Status of the condition, one of True, False, Unknown.
+	Status api.ConditionStatus
+	// The last time this condition was updated.
+	LastUpdateTime metav1.Time


Do we want to remove LastUpdateTime now? It would mean this is inconsistent with others, but at least we wouldn't propagate it.

Is there consensus on this?

@kow3ns @janetkuo thoughts?

LastUpdateTime was added to DeploymentCondition to replace LastHeartbeatTime or lastProbeTime we see in other conditions (ref #19343 (comment) and #35529 (comment)). It's useful for users to understand what is going on.

We haven't added it to ReplicaSetCondition, though.

Based on our discussion - looks like we can remove it from here, since this type isn't in use yet. There's no strong reason to add it only to remove it later.

smarterclayton · 2017-11-07T23:02:36Z

/approve

One comment

janetkuo · 2017-11-08T01:00:20Z

staging/src/k8s.io/api/apps/v1/types.go

+
+	// Represents the latest available observations of a statefulset's current state.
+	// +patchMergeKey=type
+	// +patchStrategy=merge


Need // +optional tag for every omitempty field.

ref https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#optional-vs-required

janetkuo · 2017-11-08T01:01:50Z

staging/src/k8s.io/api/apps/v1/types.go

+	// Status of the condition, one of True, False, Unknown.
+	Status v1.ConditionStatus `json:"status" protobuf:"bytes,2,opt,name=status,casttype=k8s.io/api/core/v1.ConditionStatus"`
+	// The last time this condition was updated.
+	LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty" protobuf:"bytes,6,opt,name=lastUpdateTime"`


Why's the protobuf tag 6 instead of 3? Just remove the protobuf part and let the codegen auto-generate it instead

foxish · 2017-11-08T04:36:59Z

/retest

janetkuo · 2017-11-08T18:16:29Z

/lgtm

fejta-bot · 2017-11-08T20:41:58Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

foxish · 2017-11-08T22:53:53Z

pkg/apis/apps/v1/conversion.go

@@ -465,6 +466,7 @@ func Convert_v1_StatefulSetStatus_To_apps_StatefulSetStatus(in *appsv1.StatefulS
 		out.CollisionCount = new(int32)
 		*out.CollisionCount = *in.CollisionCount
 	}
+	out.Conditions = *(*[]apps.StatefulSetCondition)(unsafe.Pointer(&in.Conditions))


@caesarxuchao PTAL. Does this seem reasonable?

As per offline discussion - simplified to not use unsafe conversions.

foxish · 2017-11-09T02:26:18Z

Rebased. @janetkuo PTAL

janetkuo · 2017-11-09T07:19:37Z

/lgtm

foxish · 2017-11-09T17:10:34Z

/retest

foxish · 2017-11-09T17:22:29Z

/test pull-kubernetes-e2e-gce-device-plugin-gpu

foxish · 2017-11-09T19:09:36Z

/test pull-kubernetes-verify
/test pull-kubernetes-e2e-gce
/test pull-kubernetes-bazel-test
/test pull-kubernetes-e2e-gce-device-plugin-gpu

kow3ns · 2017-11-09T22:49:36Z

/lgtm

k8s-github-robot · 2017-11-09T22:50:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: foxish, janetkuo, kow3ns, smarterclayton

Associated issue: 7856

The full list of commands accepted by this bot can be found here.

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

~~api/OWNERS~~ [smarterclayton]
~~docs/api-reference/OWNERS~~ [smarterclayton]
~~pkg/apis/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/api/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

foxish · 2017-11-10T01:38:49Z

/test pull-kubernetes-unit

k8s-github-robot · 2017-11-10T04:11:27Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-11-10T05:05:42Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

antoineco · 2018-01-09T11:18:35Z

xref #51594

foxish assigned janetkuo Nov 7, 2017

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 7, 2017

k8s-github-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Nov 7, 2017

foxish assigned smarterclayton Nov 7, 2017

smarterclayton reviewed Nov 7, 2017

View reviewed changes

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

foxish force-pushed the add-sts-conditions branch from 1da3518 to a3f7cfd Compare November 8, 2017 00:42

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 Nov 8, 2017

janetkuo reviewed Nov 8, 2017

View reviewed changes

foxish force-pushed the add-sts-conditions branch from a3f7cfd to ffd9fc2 Compare November 8, 2017 02:00

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

foxish force-pushed the add-sts-conditions branch 2 times, most recently from 936a528 to 3098139 Compare November 8, 2017 02:53

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 Nov 8, 2017

janetkuo approved these changes Nov 8, 2017

View reviewed changes

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

foxish force-pushed the add-sts-conditions branch from 3098139 to 5b655b4 Compare November 8, 2017 22:52

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 8, 2017

foxish commented Nov 8, 2017

View reviewed changes

foxish force-pushed the add-sts-conditions branch 2 times, most recently from 8075ca5 to 9efca4d Compare November 8, 2017 23:17

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2017

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

foxish force-pushed the add-sts-conditions branch from d2b1448 to 5400ed0 Compare November 9, 2017 20:27

foxish removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2017

Added conditions to statefulset

8258216

foxish added the queue/multiple-rebases label Nov 9, 2017

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

Autogenerated

45ab57f

foxish force-pushed the add-sts-conditions branch from 5400ed0 to 45ab57f Compare November 9, 2017 21:34

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

k8s-ci-robot assigned kow3ns Nov 9, 2017

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

k8s-github-robot merged commit 3ce7c3e into kubernetes:master Nov 10, 2017

foxish deleted the add-sts-conditions branch November 10, 2017 10:16

csrwng mentioned this pull request Dec 14, 2017

WIP - Proposal for Cluster API openshift/cluster-operator#21

Closed

antoineco mentioned this pull request Jan 9, 2018

Promote Statefulset controller and its e2e tests to use apps/v1 #55792

Merged

dependabot-preview bot mentioned this pull request May 8, 2018

Bump kubernetes from 4.0.0 to 6.0.0 ministryofjustice/analytics-platform-unidler#3

Merged

dependabot-preview bot mentioned this pull request May 16, 2018

Bump kubernetes from 3.0.0 to 6.0.0 ministryofjustice/analytics-platform-control-panel#161

Closed

ash2k mentioned this pull request Sep 26, 2018

Condition lastUpdateTime is not preserved atlassian/smith#322

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add statefulset conditions #55268

Add statefulset conditions #55268

foxish commented Nov 7, 2017 •

edited

Loading

smarterclayton Nov 7, 2017

foxish Nov 7, 2017

janetkuo Nov 8, 2017

foxish Nov 8, 2017

smarterclayton commented Nov 7, 2017

janetkuo Nov 8, 2017 •

edited

Loading

janetkuo Nov 8, 2017 •

edited

Loading

foxish commented Nov 8, 2017

janetkuo commented Nov 8, 2017

fejta-bot commented Nov 8, 2017

foxish Nov 8, 2017 •

edited

Loading

foxish Nov 8, 2017

foxish commented Nov 9, 2017

janetkuo commented Nov 9, 2017

foxish commented Nov 9, 2017

foxish commented Nov 9, 2017

foxish commented Nov 9, 2017

kow3ns commented Nov 9, 2017

k8s-github-robot commented Nov 9, 2017

foxish commented Nov 10, 2017

k8s-github-robot commented Nov 10, 2017

k8s-github-robot commented Nov 10, 2017

antoineco commented Jan 9, 2018

Add statefulset conditions #55268

Add statefulset conditions #55268

Conversation

foxish commented Nov 7, 2017 • edited Loading

smarterclayton Nov 7, 2017

Choose a reason for hiding this comment

foxish Nov 7, 2017

Choose a reason for hiding this comment

janetkuo Nov 8, 2017

Choose a reason for hiding this comment

foxish Nov 8, 2017

Choose a reason for hiding this comment

smarterclayton commented Nov 7, 2017

janetkuo Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

janetkuo Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

foxish commented Nov 8, 2017

janetkuo commented Nov 8, 2017

fejta-bot commented Nov 8, 2017

foxish Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

foxish Nov 8, 2017

Choose a reason for hiding this comment

foxish commented Nov 9, 2017

janetkuo commented Nov 9, 2017

foxish commented Nov 9, 2017

foxish commented Nov 9, 2017

foxish commented Nov 9, 2017

kow3ns commented Nov 9, 2017

k8s-github-robot commented Nov 9, 2017

foxish commented Nov 10, 2017

k8s-github-robot commented Nov 10, 2017

k8s-github-robot commented Nov 10, 2017

antoineco commented Jan 9, 2018

foxish commented Nov 7, 2017 •

edited

Loading

janetkuo Nov 8, 2017 •

edited

Loading

janetkuo Nov 8, 2017 •

edited

Loading

foxish Nov 8, 2017 •

edited

Loading