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

Add DaemonSet conditions #55272

Merged
merged 2 commits into from
Nov 9, 2017
Merged

Add DaemonSet conditions #55272

merged 2 commits into from
Nov 9, 2017

Conversation

janetkuo
Copy link
Member

@janetkuo janetkuo commented Nov 7, 2017 •
edited
Loading

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
xref kubernetes/enhancements#353, #7856

Special notes for your reviewer:
A conditions field is added to DaemonSetStatus for consistency. No valid DaemonSet condition type is supported yet.
@kubernetes/sig-apps-api-reviews

Release note:

DaemonSet status now has a new field named "conditions", making it consistent with other workloads controllers.

@janetkuo janetkuo added this to the v1.9 milestone Nov 7, 2017
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 7, 2017
@janetkuo janetkuo force-pushed the ds-conditions branch 3 times, most recently from ff98411 to 003dd0b Compare November 7, 2017 23:55
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 7, 2017
@janetkuo
Copy link
Member Author

janetkuo commented Nov 8, 2017

/test pull-kubernetes-unit

smarterclayton
smarterclayton reviewed Nov 8, 2017
View reviewed changes
staging/src/k8s.io/api/apps/v1beta2/types.go Outdated
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.
// +optional
LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty" protobuf:"bytes,3,opt,name=lastUpdateTime"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question as for stateful sets - should we omit this field since we believe it's use is limited?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. We can omit it for now.

@smarterclayton
Copy link
Contributor

/approve

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

janetkuo commented Nov 8, 2017

Removed LastUpdateTime field from DaemonSetCondition. Deployment will be the only workload API with LastUpdateTime in its Condition.

@janetkuo
Copy link
Member Author

janetkuo commented Nov 8, 2017

/test pull-kubernetes-e2e-kops-aws

@dixudx
Copy link
Member

dixudx commented Nov 8, 2017

@janetkuo pull-kubernetes-e2e-kops-aws flake #55303

@dixudx
Copy link
Member

dixudx commented Nov 8, 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 Nov 8, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

Associated issue: 353

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

@fejta-bot
Copy link

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

Review the full test history for this PR.

@janetkuo
Copy link
Member Author

janetkuo commented Nov 8, 2017

/test pull-kubernetes-e2e-kops-aws

@0xmichalis
Copy link
Contributor

/cc tnozicka

@k8s-ci-robot k8s-ci-robot requested a review from tnozicka November 8, 2017 20:46
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 55331, 55272, 55228, 49763, 55242). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit f644681 into kubernetes:master Nov 9, 2017
@csrwng csrwng mentioned this pull request Dec 14, 2017
Closed
@dependabot-preview dependabot-preview bot mentioned this pull request May 8, 2018
Merged
@dependabot-preview dependabot-preview bot mentioned this pull request May 16, 2018
Closed
@ash2k ash2k mentioned this pull request Sep 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarterclayton smarterclayton smarterclayton left review comments

@tnozicka tnozicka Awaiting requested review from tnozicka

Assignees

@foxish foxish

@smarterclayton smarterclayton

@dixudx dixudx

@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. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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
v1.9
Development

Successfully merging this pull request may close these issues.
9 participants
@janetkuo @smarterclayton @dixudx @k8s-github-robot @fejta-bot @0xmichalis @foxish @kow3ns @k8s-ci-robot