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.

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

✨ bump controller-runtime to 0.8.3 #1961

Merged
merged 1 commit into from
May 3, 2021

Conversation

varshaprasad96
Copy link
Member

deps: Bump controller-runtime to 0.8.0

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 18, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @varshaprasad96. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 18, 2021
@k8s-ci-robot k8s-ci-robot requested review from droot and estroz January 18, 2021 21:03
@varshaprasad96
Copy link
Member Author

Blocked on #146. Need to regenerate test data for project-v3-addon and project-v3-config plugins.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 30, 2021
@Adirio
Copy link
Contributor

Adirio commented Feb 10, 2021

Note: current PR title and description checks do not understand the [WIP] part, so they will fail anyway, but once you remove it, the emoji (✨ :sparkles:) is expected without brackets, so you will also have to remove those.

P.S.: the next version of the PR title verifier will understand the [WIP] prefix.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 23, 2021
@varshaprasad96 varshaprasad96 changed the title [wip] (:sparkles:) bump controller-runtime to 0.8.0 (:sparkles:) bump controller-runtime to 0.8.0 Feb 23, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 23, 2021
@varshaprasad96
Copy link
Member Author

The PR has been updated. Waiting for v3.0.0 release.
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2021
@varshaprasad96 varshaprasad96 changed the title (:sparkles:) bump controller-runtime to 0.8.0 ✨ bump controller-runtime to 0.8.0 Feb 23, 2021
@Adirio
Copy link
Contributor

Adirio commented Feb 23, 2021

@varshaprasad96 Could you remove the brackets from the title's emoji? Either ✨ or :sparkles: is fine, (and GitHub started understanding emojis on titles).

@varshaprasad96 varshaprasad96 force-pushed the bump-cr/0.8.0 branch 2 times, most recently from 4813342 to 8f99d57 Compare February 23, 2021 18:42
@camilamacedo86
Copy link
Member

camilamacedo86 commented Feb 23, 2021

HI @estroz,

You spoke about we push it in 3.1.0. However, wdyt? Could it get merged now or should we wait? if it gets merged than it means that if we need to rebase master with the release-v3 before 3.0.0 it will ship as well. I have not an objection at all with 3.0.0 be using controller-runtime 0.8.0.

c/c @Adirio

@Adirio
Copy link
Contributor

Adirio commented Feb 23, 2021

The reason that was given (I think it was @estroz but not 100% sure) was that there is no kubebuilder version that supports v0.7.0 which is the first version supporting kubernetes 1.19.x so that it would be nicer to have a v3.0.0 with v0.7.0 and then upgrade it for v3.1.0.

Copy link
Contributor

@Adirio Adirio left a comment

Choose a reason for hiding this comment

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

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 24, 2021
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

I am ok with it get merged. I do not think that would be a requirement we have any version of the tool using 0.7.0.

@varshaprasad96 could you only please update the migration doc and bump the version 0.8.0 where we have 0.7.0?

c/c @estroz

@camilamacedo86 camilamacedo86 self-requested a review February 24, 2021 20:23
@varshaprasad96 varshaprasad96 force-pushed the bump-cr/0.8.0 branch 2 times, most recently from 6dd78ad to f70350d Compare February 25, 2021 17:58
@varshaprasad96
Copy link
Member Author

Based on the discussion, I am reverting the migration doc changes, since the default version for v3 stable would be c-r 0.7.0

@estroz
Copy link
Contributor

estroz commented Feb 25, 2021

since the default version for v3 stable would be c-r 0.7.0

@varshaprasad96 yes but after this PR merges it will be 0.8.0 right?

@Adirio
Copy link
Contributor

Adirio commented Feb 25, 2021

Even if the docs are changed in master they won't get reflected in the published book until a new release that contains this merged PR is made, and then the version would be 0.8.0.

@camilamacedo86
Copy link
Member

@varshaprasad96 @estroz,

This PR bumps the controller-runtime to 0.8.0
When it gets merged we need also update the docs
The production docs are only updated as part of the release where we push the master changes to the book branch. So, the master docs need to reflect the master state and update the doc is part of the PR. If we need to revert we will also revert the docs changes.

Is it make sense?

@varshaprasad96
Copy link
Member Author

Sorry, got confused ! Updated the migration docs

@camilamacedo86 camilamacedo86 added this to the 3.1.0 milestone Mar 1, 2021
@camilamacedo86
Copy link
Member

We need also have the commits squashed as well.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2021
@estroz
Copy link
Contributor

estroz commented Apr 30, 2021

@varshaprasad96 this can be rebased and bumped to controller-runtime v0.8.3

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 3, 2021
@varshaprasad96 varshaprasad96 changed the title ✨ bump controller-runtime to 0.8.0 ✨ bump controller-runtime to 0.8.3 May 3, 2021
@varshaprasad96
Copy link
Member Author

@estroz Done. bumped to controller-runtime v0.8.3

@estroz
Copy link
Contributor

estroz commented May 3, 2021

/approve
/lgtm
/hold cancel

@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 May 3, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Adirio, estroz, varshaprasad96

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
Copy link
Contributor

@varshaprasad96: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubebuilder-e2e-k8s-1-19-4 12f9460 link /test pull-kubebuilder-e2e-k8s-1-19-4

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.

@estroz
Copy link
Contributor

estroz commented May 3, 2021

/retest

@k8s-ci-robot k8s-ci-robot merged commit 2b857cb into kubernetes-sigs:master May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants