Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

StatefulSet: Remove pod.alpha.kubernetes.io/initialized annotation. #49251

Merged
merged 2 commits into from
Jul 25, 2017
Merged

StatefulSet: Remove pod.alpha.kubernetes.io/initialized annotation. #49251

merged 2 commits into from
Jul 25, 2017

Conversation

enisoc
Copy link
Member

@enisoc enisoc commented Jul 20, 2017

The pod.alpha.kubernetes.io/initialized annotation was originally a tool for validating StatefulSet's ordered Pod creation guarantees during the feature's alpha phase.

If set to "false" on a given Pod, it would interrupt StatefulSet's normal behavior. In v1.5.0, the annotation was deprecated and the default became "true" as part of StatefulSet's graduation to beta.

The annotation is now ignored, meaning it cannot be used to interrupt StatefulSet Pod management.

StatefulSet: The deprecated `pod.alpha.kubernetes.io/initialized` annotation for interrupting StatefulSet Pod management is now ignored. If you were setting it to `true` or leaving it unset, no action is required. However, if you were setting it to `false`, be aware that previously-dormant StatefulSets may become active after upgrading.

ref #41605

@enisoc enisoc added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/apps Categorizes an issue or PR as relevant to SIG Apps. labels Jul 20, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 20, 2017
@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 20, 2017
@enisoc
Copy link
Member Author

enisoc commented Jul 20, 2017

/test pull-kubernetes-e2e-gce-etcd3

@enisoc enisoc mentioned this pull request Jul 20, 2017
Merged
kow3ns
kow3ns approved these changes Jul 21, 2017
View reviewed changes
Copy link
Member

@kow3ns kow3ns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is exactly what I had in mind as well. Pausing the StatefulSet, for testing purposes, by breaking its readiness probe achieves the exact same result as the removed annotation without unnecessarily exposing implementation details, and will meet the requirements of backward compatibility.

  1. Do you intend to cherry pick b8214b2 into 1.7 e2e so that upgrade tests will function?
  2. Also we should consider removing the annotation from the v1beta2 API.

Both of the above can be addressed in future PRs.

@kow3ns
Copy link
Member

kow3ns commented Jul 21, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2017
@kow3ns
Copy link
Member

kow3ns commented Jul 21, 2017

/approve

@enisoc enisoc mentioned this pull request Jul 21, 2017
Closed
@enisoc
Copy link
Member Author

enisoc commented Jul 21, 2017

@kow3ns Good points. I've added TODO items to #41605 (comment).

enisoc added 2 commits July 24, 2017 10:18
@enisoc
StatefulSet: Stop using initialized annotation in e2e tests.
48e8370 
Instead, use Readiness to pause and resume individual Pods to verify
that the StatefulSet controller performs the right actions one at a
time.
@enisoc
StatefulSet: Remove pod.alpha.kubernetes.io/initialized annotation.
283211c
@enisoc enisoc force-pushed the initialized-annotation branch from 4122a70 to 283211c Compare July 24, 2017 17:18
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 24, 2017
@enisoc enisoc added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 24, 2017
@enisoc
Copy link
Member Author

enisoc commented Jul 24, 2017

I'm waiting for #49400 before assigning for approval.

@smarterclayton
Copy link
Contributor

/approve

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

Associated issue: 41605

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

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

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

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2017
@enisoc
Copy link
Member Author

enisoc commented Jul 24, 2017

/retest

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 48636, 49088, 49251, 49417, 49494)

@k8s-github-robot k8s-github-robot merged commit 7e5fd72 into kubernetes:master Jul 25, 2017
@enisoc enisoc mentioned this pull request Jul 26, 2017
Merged
k8s-github-robot pushed a commit that referenced this pull request Jul 31, 2017
Merge pull request #49651 from enisoc/v1beta2-initialized
0acb503 
Automatic merge from submit-queue (batch tested with PRs 49651, 49707, 49662, 47019, 49747)

StatefulSet: Remove `initialized` annotation from apps/v1beta2.

The annotation was already removed from apps/v1beta1 in #49251, but this copy survived due to another concurrent PR.

ref #41605
@enisoc enisoc deleted the initialized-annotation branch August 23, 2017 23:06
@crimsonfaith91 crimsonfaith91 mentioned this pull request Sep 20, 2017
Closed
@zouyee zouyee mentioned this pull request Sep 26, 2017
Merged
This was referenced Sep 26, 2017
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kow3ns kow3ns kow3ns approved these changes

Assignees

@kow3ns kow3ns

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/apps Categorizes an issue or PR as relevant to SIG Apps. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@enisoc @kow3ns @smarterclayton @k8s-github-robot @k8s-ci-robot