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

deps: bump to controller-runtime v0.7.0 #1746

Closed
3 tasks done
estroz opened this issue Oct 27, 2020 · 10 comments
Closed
3 tasks done

deps: bump to controller-runtime v0.7.0 #1746

estroz opened this issue Oct 27, 2020 · 10 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@estroz
Copy link
Contributor

estroz commented Oct 27, 2020

This issue tracks the v0.7.0 controller-runtime bump, which has breaking changes. Also bumps deps to k8s
v1.19.

/kind feature

@estroz estroz added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 27, 2020
@estroz
Copy link
Contributor Author

estroz commented Oct 27, 2020

/assign

@camilamacedo86
Copy link
Member

@estroz IMO we might need to wait for the stable release to be able to bump the dep.

@estroz
Copy link
Contributor Author

estroz commented Oct 27, 2020

From kubernetes-sigs/kubebuilder-declarative-pattern#127 (comment) (applies here too):

Is there a good reason to wait? I don't expect any changes to go in aside from bugfixes at this point, although it is technically an alpha release. Unless there is a plan to release this project soon, bumping to alpha now then bumping to stable before a release makes sense to me since the alpha bump will only ever be on master. This also helps test out the controller-runtime v0.7.0 release.

Also we are currently pinning to unreleased code with sigs.k8s.io/kubebuilder-declarative-pattern, so I don't see why we can't do something similar for controller-runtime, but with a tagged release.

@estroz
Copy link
Contributor Author

estroz commented Oct 27, 2020

@pwittrock @DirectXMan12 @droot how do you feel about pinning to commits/alpha|beta releases in master in general?

@estroz
Copy link
Contributor Author

estroz commented Oct 28, 2020

I'll also mention that v0.6.3 has a logging bug in it (fixed by kubernetes-sigs/controller-runtime#972) which breaks kubebuilder e2e tests (see this comment). Not sure why the tests passed on #1721 See below for why CI e2e tests are still passing.

We can either bump to v0.7.0 or get a patch release v0.6.4 going, then bump here. I suggest going straight to v0.7.0.

@camilamacedo86
Copy link
Member

camilamacedo86 commented Oct 29, 2020

Hi @estroz,

Following some comments.

Also we are currently pinning to unreleased code with sigs.k8s.io/kubebuilder-declarative-pattern

This project is used only for the addon and it does not work with release tags

I'll also mention that v0.6.3 has a logging bug in it (fixed by kubernetes-sigs/controller-runtime#972) which breaks kubebuilder e2e tests (see this comment). Not sure why the tests passed on #1721.

The tests in the PR #1644 is not passing, however, I do not think so that it is the root cause. See that it only not works in your PR. So, I am not sure about it. I think we need re-check its implementation. Did you try to bump the alpha version in your PR to see if it will fix?

Regards bump to controller-runtime v0.7.0:

So, far I think we just bump stable versions. I would prefer not to bump alpha versions. How, can we release the CLI scaffolding projects with an alpha version of controller-runtime? Would it really be a good approach? Also, did you check if controller-runtime v0.7.0 has breaking changes for kb? If yes, should not we apply it only for v3+ plugin? PS. I did not check it further, however, shows that it might have a breaking change for the scaffolds. See; https://kubernetes.slack.com/archives/CAR30FCJZ/p1603499651205900

@estroz
Copy link
Contributor Author

estroz commented Oct 29, 2020

it does not work with release tags

Interesting, I didn't notice that repo hasn't been tagged.

The tests in the PR #1644 is not passing, however, I do not think so that it is the root cause

Once I pinned controller-runtime to a version containing this change that I created locally, e2e tests passed. If you look at the log output for an operator built with kubebuilder master with a go/v3-alpha project, you never see "Successfully Reconciled" because debug logging isn't working correctly. This only affects go/v3-alpha, which isn't tested in e2e tests.

How, can we release the CLI scaffolding projects with an alpha version of controller-runtime

We can't and shouldn't. However if we want to block a release on bumping to the next major/minor version of controller-runtime, which we probably do for go/v3-alpha in this case for a few reasons, then bumping to alpha on master is fine.

Also, did you check if controller-runtime v0.7.0 has breaking changes for kb? If yes, should not we apply it only for v3+ plugin

It does, and yes only go/v3-alpha should get this bump.

@camilamacedo86
Copy link
Member

HI @estroz,

So, I think it is all fine. +1 to do the bump. I agree that if we bump only go/v3-alpha also would be ok to do the release after that.
Could we just have an issue tracked to the milestone 3.0.0 for we remember and try to bump the stable version before its release?

@estroz
Copy link
Contributor Author

estroz commented Oct 29, 2020

Awesome. I would go even further to say that any -alpha plugin can get an alpha dependency version, but cannot be stabilized until a stable dependency version is released and the dependency bumped to that version.

@camilamacedo86
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

2 participants