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

Bump to controller-runtime 0.3.0 and Kubernetes 1.15 #2083

Merged
merged 6 commits into from
Oct 24, 2019
Merged

Bump to controller-runtime 0.3.0 and Kubernetes 1.15 #2083

merged 6 commits into from
Oct 24, 2019

Conversation

joelanford
Copy link
Member

@joelanford joelanford commented Oct 19, 2019 •
edited
Loading

Description of the change:
Bump dependencies:

  • controller-runtime to v0.3.0
  • kubernetes to v1.15.4
  • controller-tools to v0.2.2

Motivation for the change:

  • To support new features from controller-runtime, controller-tools, and kubernetes

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 19, 2019
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 19, 2019
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 20, 2019
@joelanford joelanford changed the title WIP: Bump to controller-runtime 0.3.0 and Kubernetes 1.15 Bump to controller-runtime 0.3.0 and Kubernetes 1.15 Oct 21, 2019
@openshift-ci-robot openshift-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 Oct 21, 2019
@joelanford joelanford mentioned this pull request Oct 21, 2019
Closed
camilamacedo86
camilamacedo86 approved these changes Oct 21, 2019
View reviewed changes
Copy link
Contributor

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

It is /lgtm for me.
All shows great. It is just missing the CHANGELOG.
/approved

@joelanford
Copy link
Member Author

/hold

Waiting for #1949 to merge.

@camilamacedo86 good catch on the CHANGELOG. I'll update it.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 21, 2019
joelanford added a commit to joelanford/operator-sdk that referenced this pull request Oct 21, 2019
@joelanford
CHANGELOG.md: add lines for operator-framework#2083
1a8d2d1
@joelanford
Copy link
Member Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 23, 2019
@joelanford
Copy link
Member Author

/test e2e-aws-ansible
/test e2e-aws-go
/test e2e-aws-helm

@joelanford joelanford mentioned this pull request Oct 24, 2019
Closed
@joelanford
Copy link
Member Author

/test marker

hasbro17
hasbro17 reviewed Oct 24, 2019
View reviewed changes
CHANGELOG.md
- Upgrade Kubernetes version from `kubernetes-1.14.1` to `kubernetes-1.15.4`. ([#2083](https://github.com/operator-framework/operator-sdk/pull/2083))
- Upgrade Helm version from `v2.14.1` to `v2.15.0`. ([#2083](https://github.com/operator-framework/operator-sdk/pull/2083))
- Upgrade [`controller-runtime`](https://github.com/kubernetes-sigs/controller-runtime) version from `v0.2.0` to `v0.3.0`. ([#2083](https://github.com/operator-framework/operator-sdk/pull/2083))
- Upgrade [`controller-tools`](https://github.com/kubernetes-sigs/controller-tools) version from `v0.2.1+git` to `v0.2.2`. ([#2083](https://github.com/operator-framework/operator-sdk/pull/2083))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be communicated as a breaking change to SDK users? Or even be in the CHANGELOG at all?

I don't think they're expected to use controller-tools as a dependency.
https://github.com/operator-framework/operator-sdk/blob/master/internal/scaffold/go_mod.go#L38-L69

That's more for the SDK CLI's internal use.

Copy link
Member Author

Choose a reason for hiding this comment

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

It isn't listed as a breaking change unless I screwed up the markdown. I included it because it has some fixes that affect CRD generation.

I could either remove it altogether or add a link to the v0.2.2 tag to link it to the controller-tools release notes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah you're right. I misread the raw markdown for the prefix **Breaking change:** and thought this was it's own section like we had in the last release.

But yeah we should link to the release notes if we want to highlight the CRD generation bug fixes.
Same for the controller-runtime release notes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Because to (most?) SDK users who aren't using controller-tools directly it won't be clear why this version bump is important.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I’ll make a follow-up with this and @estroz’s suggestion.

hasbro17
hasbro17 approved these changes Oct 24, 2019
View reviewed changes
Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

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

LGTM

estroz
estroz approved these changes Oct 24, 2019
View reviewed changes
Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

Need to make a follow-up to update kubernetes 1.14 references to 1.15

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2019
@joelanford joelanford merged commit 17d3890 into operator-framework:master Oct 24, 2019
joelanford added a commit to joelanford/operator-sdk that referenced this pull request Oct 24, 2019
@joelanford
*: updating k8s version references, follow-up from operator-framework…
93e2c7b 
…#2083
@joelanford joelanford deleted the k8s-1.15 branch October 24, 2019 23:25
@joelanford joelanford mentioned this pull request Oct 24, 2019
Merged
joelanford added a commit that referenced this pull request Oct 28, 2019
@joelanford
*: updating k8s version references, follow-up from #2083 (#2109)
12112ad 
* *: updating k8s version references, follow-up from #2083

* test/e2e/_incluster-test-code/memcached_test.go: metrics check won't fully pass until k8s 1.16+
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estroz estroz estroz approved these changes

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@hasbro17 hasbro17 hasbro17 approved these changes

Assignees

@estroz estroz

Labels
lgtm Indicates that a PR is ready to be merged. 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.
5 participants
@joelanford @hasbro17 @camilamacedo86 @estroz @openshift-ci-robot