Bump to Go 1.19, Ginkgo v2 & kubebuilder 3.7.1#6047
Bump to Go 1.19, Ginkgo v2 & kubebuilder 3.7.1#6047laxmikantbpandhare merged 2 commits intooperator-framework:masterfrom
Conversation
laxmikantbpandhare
force-pushed
the
ginkgo-v2
branch
from
d3c1d26 to
31ad5bf
Compare
|
Updated the |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
hack/generate/samples/internal/go/memcached-with-webhooks/memcached_with_webhooks.go
Outdated
Show resolved
Hide resolved
testdata/ansible/memcached-operator/config/default/kustomization.yaml
Outdated
Show resolved
Hide resolved
testdata/helm/memcached-operator/config/default/kustomization.yaml
Outdated
Show resolved
Hide resolved
|
/lgtm |
everettraven
left a comment
everettraven
left a comment
There was a problem hiding this comment.
A couple things:
- We need to update the changelog to include entries for everything that is a user facing change and may require a migration guide.
- I would like add comments to the helm/ansible plugins where we are replacing code because of the yaml linting error. Ideally this will be fixed in Kubebuilder at some point and we won't need to do this replace anymore.
- The ansible/helm samples in the
testdata/directory have many files deleted from themanifests/directory. Why is this being done?
There may be a couple other little nits, but the above summary is the major things that I think we need to address before merging this in.
testdata/ansible/memcached-operator/bundle/manifests/cache.example.com_memcacheds.yaml
Show resolved
Hide resolved
testdata/helm/memcached-operator/bundle/manifests/cache.example.com_memcacheds.yaml
Show resolved
Hide resolved
|
Saw a typo in generation code, it's been there a while so I'm not going to fix it in this PR. #6085 |
jmrodri
left a comment
jmrodri
left a comment
There was a problem hiding this comment.
Great PR! That was a lot of work. A few things I think we need to add to this (this is a short summary, use the comments in the review for all of the actual details) :)
- update PR description to give a summary of all of the changes in this PR. You bumped go, ginkgo, and kube. You had to change some Labels to LabelsMap, explain why that had to happen. Mention some of the repercussions the kube bump had with component config.
- I think we need a couple changelog files. Might be easier to capture the migrations for the specific ones if they are in separate files
- Changes to ansible scaffolding, with migration
- Changes to helm scaffolding, with migration
- Changes to go dependencies, with migration; also call out the multiarch change. That might have to be a migration step as well.
- Component Config needs to be mentioned somewhere, either in the ansible and helm or separately
- go, ginkgo, kubebuilder bumps can be in a single changelog fragment
hack/generate/samples/internal/go/memcached-with-webhooks/e2e_test_code.go
Show resolved
Hide resolved
|
Added changelogs below:
|
Signed-off-by: laxmikantbpandhare <[email protected]> updated kb version to get ginkgo/v2 changes Signed-off-by: laxmikantbpandhare <[email protected]> added changelog Signed-off-by: laxmikantbpandhare <[email protected]> By() cleause used outside and it was throwing error updated go.mod depedencies too many arguments InstallCertManager and UnInstallCertManager update kb go.mod updated --leader-elec skip in the changes kb changes fort envtest updated testdata update go version 1.19 and component config added to moulecule test updated go version in github actions and docker sdk images updated io/ioutil updated all io/ioutil to io or os as it is deprecated modified code for component config check modified code for component config check resolve conflicts make generate updated Signed-off-by: laxmikantbpandhare <[email protected]> updated logic Signed-off-by: laxmikantbpandhare <[email protected]> updated milecule test Signed-off-by: laxmikantbpandhare <[email protected]> removed ginkgo old version and updated ansible molecule test Signed-off-by: laxmikantbpandhare <[email protected]> updated the changes with the help of Bryce and Jesus Signed-off-by: laxmikantbpandhare <[email protected]> updated manager.yaml removes space Signed-off-by: laxmikantbpandhare <[email protected]> removed --component-config=true from webhook testcase as well Signed-off-by: laxmikantbpandhare <[email protected]> updated testdata and added changelog with migration guide Signed-off-by: laxmikantbpandhare <[email protected]> lint error for header in migration guide Signed-off-by: laxmikantbpandhare <[email protected]> added pipe | in changelog
|
/hold I'm still reviewing the latest changes. |
varshaprasad96
left a comment
varshaprasad96
left a comment
There was a problem hiding this comment.
@laxmikantbpandhare great work! Thanks for handling this big chunk of updates. Just a few changes in changeling fragments - It would be nice to include a snippet of git diff in migration sections, so that its easier for users to know what is getting added and/or deleted. Rest of the changes in SDK and plugins look good!
jmrodri
left a comment
jmrodri
left a comment
There was a problem hiding this comment.
The changelog is meant to convey what things have changed that would affect a user's project. They don't care about changes we made to internal code.
How will go 1.19 update impact a users project? the io/ioutil is a good example.
How will ginkgo v2 affect them? They dont' care that we had to rename an internal variable like LabelsMap. They care that they might have to change their suite_test.go. So look at the difference of the testdata changes. THAT is usually what they care to see. So if the testdata changes the Makefile, what changed? That is what they will want to see.
Also, migrations are only important if it affects the scaffolded out project. If the change has no effect then no need for a migration.
I know it's not easy but you'll want to think about each of these as you write the changelogs. Usually the changelogs are easy because the changes in the PR aren't that big. But this is a HUGE PR and a lot changed :)
|
/hold cancel I finished my review |
Signed-off-by: laxmikantbpandhare <[email protected]>
7 participants
Description of the change:
Changes in this PR are:
config-genis completely removed from Kubebuilder. As part of this bump, the website content of SDK was updated where the doc is referring toconfig-gen.io/ioutilimported packages to eitheriooros.io/ioutilgot deprecated in 1.16 and no longer it is used. Updated all the files in SDK.--component-configflag changes. Previously, it was scaffolding and doing some file changes without considering the--component-configflag.--component-configflag.--component-configflag changes. Previously, it was scaffolding and doing some file changes without considering the--component-configflag.Motivation for the change:
operator-sdk.ginkgoupdated toginkgo/v2io/ioutiltoioorocasio/ioutilgot deprecated in1.161.19Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs