Skip to content

Move feature gate package from k8s.io/apiserver to k8s.io/component-base #74021

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
Merged
merged 5 commits into from
May 8, 2019
Merged

Move feature gate package from k8s.io/apiserver to k8s.io/component-base #74021

merged 5 commits into from
May 8, 2019

Conversation

andrewsykim
Copy link
Member

@andrewsykim andrewsykim commented Feb 13, 2019 •
edited
Loading

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
As per discussion in https://github.com/kubernetes/kubernetes/pull/73604/files#r252809685, proposing to move k8s.io/apiserver/pkg/util/feature to k8s.io/component-base. I'm not set on the package location k8s.io/component-base/pkg/feature so happy to change it if there are better ideas. I also think this package might be better renamed to featuregate and we can switch the import alias from utilfeature to featuregate but maybe for another PR?

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

/assign @sttts @liggitt @lavalamp @luxas @mtaufen @stewart-yu

@k8s-ci-robot k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Feb 13, 2019
@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Feb 13, 2019
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 13, 2019
@k8s-ci-robot k8s-ci-robot added area/apiserver area/kubeadm area/kubelet area/release-eng Issues or PRs related to the Release Engineering subproject kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 13, 2019
liggitt
liggitt reviewed May 8, 2019
View reviewed changes
staging/src/k8s.io/cloud-provider/Godeps/Godeps.json Outdated
Copy link
Member

Choose a reason for hiding this comment

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

drop Godeps.json

@liggitt liggitt mentioned this pull request May 8, 2019
Merged
@liggitt
Copy link
Member

liggitt commented May 8, 2019

there's still godeps.json changes interleaved with the commits that need dropping

@andrewsykim andrewsykim force-pushed the move-features-component-base branch from 2797a75 to e6fd153 Compare May 8, 2019 01:36
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2019
@andrewsykim andrewsykim force-pushed the move-features-component-base branch 3 times, most recently from 7021f4d to 5d94f1c Compare May 8, 2019 01:55
@sttts
Copy link
Contributor

sttts commented May 8, 2019

integration is red

@logicalhan
Copy link
Member

integration is red

It's due to this: #77573

@sttts
Copy link
Contributor

sttts commented May 8, 2019

/retest

sttts
sttts reviewed May 8, 2019
View reviewed changes
staging/publishing/rules.yaml Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove apiserver?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member Author

@andrewsykim andrewsykim May 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

we actually don't need component-base here either anymore due to #77564

sttts
sttts reviewed May 8, 2019
View reviewed changes
staging/publishing/import-restrictions.yaml Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove apiserver?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member Author

@andrewsykim andrewsykim May 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

we actually don't need component-base here either anymore due to #77564

@luxas
Copy link
Member

luxas commented May 8, 2019

@andrewsykim this one needs a rebase. Otherwise, after @sttts' comments are addressed, we can merge

@andrewsykim andrewsykim force-pushed the move-features-component-base branch from 5d94f1c to dd88d00 Compare May 8, 2019 14:04
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented May 8, 2019 •
edited
Loading

@andrewsykim: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e-containerized d4fd1db48238873029af52041c9dc092e846ba23 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-godeps b175eba127c97cf3252b97a54307fe0adea62bf5 link /test pull-kubernetes-godeps

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@andrewsykim
run update-vendor.sh
33b0b6b 
Signed-off-by: Andrew Sy Kim <kiman@vmware.com>
@andrewsykim andrewsykim force-pushed the move-features-component-base branch from dd88d00 to 7c44b46 Compare May 8, 2019 14:07
andrewsykim added 2 commits May 8, 2019 10:12
@andrewsykim
remove apiserver deps to k8s.io/cloud-provider in publish-bot verify …
6b9b58d 
…rules

Signed-off-by: Andrew Sy Kim <kiman@vmware.com>
@andrewsykim
remove apiserver import restrictions for k8s.io/cloud-provider
dcad650 
Signed-off-by: Andrew Sy Kim <kiman@vmware.com>
@andrewsykim andrewsykim force-pushed the move-features-component-base branch from 7c44b46 to dcad650 Compare May 8, 2019 14:13
@liggitt
Copy link
Member

liggitt commented May 8, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim, liggitt, rosti, sttts

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 8, 2019
@andrewsykim
Copy link
Member Author

/retest

@k8s-ci-robot k8s-ci-robot merged commit 09c4e10 into kubernetes:master May 8, 2019
@liggitt liggitt removed the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sttts sttts sttts left review comments

@cheftako cheftako cheftako left review comments

@liggitt liggitt liggitt left review comments

@a-robinson a-robinson Awaiting requested review from a-robinson

@anguslees anguslees Awaiting requested review from anguslees

+2 more reviewers

@rosti rosti rosti approved these changes

@luxas luxas luxas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lavalamp lavalamp

@sttts sttts

@mtaufen mtaufen

@cheftako cheftako

@liggitt liggitt

@luxas luxas

@stewart-yu stewart-yu

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/cloudprovider area/dependency Issues or PRs related to dependency changes area/kubeadm area/kubelet area/release-eng Issues or PRs related to the Release Engineering subproject area/test 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 kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

v1.15

Development

Successfully merging this pull request may close these issues.