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

Moved staging/src/k8s.io/client-go/tools/bootstrap to staging/src/k8s… #67356

Merged
merged 1 commit into from Oct 3, 2018
Merged

Moved staging/src/k8s.io/client-go/tools/bootstrap to staging/src/k8s… #67356

merged 1 commit into from Oct 3, 2018

Conversation

yliaog
Copy link
Contributor

@yliaog yliaog commented Aug 14, 2018 •
edited by sttts

What this PR does / why we need it:
Moved bootstrap out of client-go, where it does not belong to.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #64627

Special notes for your reviewer:

Dev-Release note:

action required: client-go release will no longer have bootstrap (k8s.io/client-go/tools/bootstrap) related code. any reference to it will break. Please redirect all references to k8s.io/bootstrap instead.

@k8s-ci-robot k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. 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. area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Aug 14, 2018
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 15, 2018
@yliaog yliaog force-pushed the master branch 6 times, most recently from 0a2ffa0 to c6eda5d Compare August 17, 2018 19:50
@yliaog
Copy link
Contributor Author

yliaog commented Aug 17, 2018

/assign @caesarxuchao

@yliaog
Copy link
Contributor Author

yliaog commented Aug 17, 2018

/test pull-kubernetes-e2e-kops-aws

@yliaog
Copy link
Contributor Author

yliaog commented Aug 17, 2018

/assign @sttts

sttts
sttts reviewed Aug 20, 2018
View reviewed changes
hack/import-restrictions.yaml Outdated
@@ -61,7 +61,7 @@
- baseImportPath: "./vendor/k8s.io/client-go/tools/"
excludeTests: true
ignoredSubTrees:
- "./vendor/k8s.io/client-go/tools/bootstrap/token/api"
- "./vendor/k8s.io/bootstrap/token/api"
Copy link
Contributor

Choose a reason for hiding this comment

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

this line can go.

But we need another similar block for the whole k8s.io/bootstrap directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kubernetes/publishing-bot#89

Copy link
Contributor Author

Choose a reason for hiding this comment

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

06a3a3b

@sttts
Copy link
Contributor

sttts commented Aug 20, 2018

Please open a PR against https://github.com/kubernetes/publishing-bot/blob/master/configs/kubernetes-rules-configmap.yaml with the new repo.

sttts
sttts reviewed Aug 20, 2018
View reviewed changes
plugin/pkg/auth/authenticator/token/bootstrap/bootstrap.go Outdated
@@ -150,7 +150,7 @@ func (t *TokenAuthenticator) AuthenticateToken(token string) (user.Info, bool, e
}, true, nil
}

// Copied from k8s.io/client-go/tools/bootstrap/token/api
// Copied from k8s.io/bootstrap/token/api
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this copied?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm not exactly sure why it was copied before. this PR moves the files from client-go to bootstrap, without touching the files' functions. i would leave it to the original author to fix the copy if necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

@luxas @timstclair you probably know

@yliaog yliaog mentioned this pull request Aug 20, 2018
Merged
@yliaog
Copy link
Contributor Author

yliaog commented Aug 20, 2018

kubernetes/publishing-bot#89 for adding the bootstrap repo

@yliaog
Copy link
Contributor Author

yliaog commented Aug 20, 2018

/test pull-kubernetes-e2e-kops-aws

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2018
@fabriziopandini fabriziopandini mentioned this pull request Sep 18, 2018
Closed
@yliaog yliaog mentioned this pull request Sep 21, 2018
Closed
1 task
@dims
Copy link
Member

dims commented Sep 24, 2018

/hold cancel

(please feel free to add /lgtm for this to merge)

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 24, 2018
@spiffxp
Copy link
Member

spiffxp commented Sep 27, 2018

/retest

@yliaog
Copy link
Contributor Author

yliaog commented Sep 27, 2018

@timothysc @luxas it looks the lgtm label was removed, do you mind to take a look again?

@yliaog
Copy link
Contributor Author

yliaog commented Sep 27, 2018

/retest

@yliaog
Copy link
Contributor Author

yliaog commented Sep 27, 2018

/test pull-kubernetes-verify

timothysc
timothysc approved these changes Oct 1, 2018
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

/lgtm

/cc @fabriziopandini

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 1, 2018
@yliaog
Copy link
Contributor Author

yliaog commented Oct 1, 2018

/retest

@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.

Silence the bot with an /lgtm cancel comment for consistent failures.

@cblecker
Copy link
Member

cblecker commented Oct 2, 2018

/retest

@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.

Silence the bot with an /lgtm cancel comment for consistent failures.

4 similar comments
@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.

Silence the bot with an /lgtm cancel comment for consistent failures.

@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.

Silence the bot with an /lgtm cancel comment for consistent failures.

@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.

Silence the bot with an /lgtm cancel comment for consistent failures.

@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.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 2, 2018
@yliaog
Copy link
Contributor Author

yliaog commented Oct 2, 2018

/retest

@yliaog
Copy link
Contributor Author

yliaog commented Oct 2, 2018

again, @timothysc @luxas it looks the lgtm label was removed, do you mind to take a look again?

@timothysc
Copy link
Member

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: caesarxuchao, jbeda, luxas, sttts, timothysc, yliaog

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 merged commit c179a9c into kubernetes:master Oct 3, 2018
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

@caesarxuchao caesarxuchao caesarxuchao left review comments

@timothysc timothysc timothysc approved these changes

@luxas luxas luxas approved these changes

@deads2k deads2k Awaiting requested review from deads2k

@fabriziopandini fabriziopandini Awaiting requested review from fabriziopandini

@liztio liztio Awaiting requested review from liztio

Assignees

@jbeda jbeda

@timothysc timothysc

@lavalamp lavalamp

@sttts sttts

@caesarxuchao caesarxuchao

@luxas luxas

@bgrant0607 bgrant0607

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm 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/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Find a new home for bootstrap tokens consts & utilities