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

Update peerpodconfig-ctl dep #342

Merged
merged 1 commit into from Sep 26, 2023
Merged

Update peerpodconfig-ctl dep #342

merged 1 commit into from Sep 26, 2023

Conversation

bpradipt
Copy link
Contributor

@bpradipt bpradipt commented Sep 20, 2023

Also update deps to match operator-sdk v1.28.0

Fixes: #KATA-2271

@bpradipt bpradipt requested review from snir911 and jensfr and removed request for littlejawa and gkurz September 20, 2023 07:56
Copy link
Contributor

@snir911 snir911 left a comment

Choose a reason for hiding this comment

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

LGTM, nit, s/ctl/ctrl/ in subject

@gkurz
Copy link
Member

gkurz commented Sep 20, 2023

Also update deps to match operator-sdk v1.28.0

@bpradipt does this mean that operator-sdk v1.28.0 should be used to build now ?

@bpradipt
Copy link
Contributor Author

@gkurz yes. I aligned the deps to the sdk version closest to existing deps

https://github.com/operator-framework/operator-sdk/blob/v1.27.0/go.mod

k8s.io/apimachinery v0.25.3
k8s.io/cli-runtime v0.25.3
k8s.io/client-go v0.25.3
k8s.io/kubectl v0.25.3
sigs.k8s.io/controller-runtime v0.13.0
sigs.k8s.io/controller-tools v0.10.0

https://github.com/operator-framework/operator-sdk/blob/v1.28.0/go.mod

k8s.io/api v0.26.2
k8s.io/apiextensions-apiserver v0.26.2
k8s.io/apimachinery v0.26.2
k8s.io/cli-runtime v0.26.2
k8s.io/client-go v0.26.2
k8s.io/kubectl v0.26.2
sigs.k8s.io/controller-runtime v0.14.5
sigs.k8s.io/controller-tools v0.11.3

Existing code

k8s.io/api v0.26.0
k8s.io/apimachinery v0.26.0
k8s.io/client-go v0.26.0
k8s.io/kubernetes v1.26.0
sigs.k8s.io/controller-runtime v0.14.1
sigs.k8s.io/controller-tools v0.10.0

@gkurz
Copy link
Member

gkurz commented Sep 20, 2023

@gkurz yes. I aligned the deps to the sdk version closest to existing deps

This means we're bumping from sdk v1.26.1 to v1.28.0. The following upgrade guidelines seem to indicate that we don't need to do any other specific change :

  1. https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.27.0/
  2. https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.28.0/

@bpradipt can you upgrade the sdk version in docs/DEVELOPMENT.md as well ?

@bpradipt
Copy link
Contributor Author

@gkurz yes. I aligned the deps to the sdk version closest to existing deps

This means we're bumping from sdk v1.26.1 to v1.28.0. The following upgrade guidelines seem to indicate that we don't need to do any other specific change :

  1. https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.27.0/
  2. https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.28.0/

Right.

@bpradipt can you upgrade the sdk version in docs/DEVELOPMENT.md as well ?
Yes

I'll also build a catalog so that we can do a pre-merge testing.

@bpradipt bpradipt added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2023
@bpradipt
Copy link
Contributor Author

Catalog: quay.io/bpradipt/openshift-sandboxed-containers-operator-catalog:v1.4.0

This results in some cascading issues and since the bulk of the changes
are in go.mod this is done in the same commit.

1. Update deps to match operator-sdk v1.28.0
   The existing deps didn't match any operator-sdk version. sdk v1.28.0
   was the closest

2. Remove unncessary replace directives
   All the required dep versions are either direct or indirect. Having
   the same deps in replace makes upgrading the deps a challenge

3. Fix the following error:

"/go/pkg/mod/k8s.io/client-go@v0.26.2/applyconfigurations/meta/v1/unstructured.go:64:38:
cannot use doc (variable of type
*"github.com/google/gnostic/openapiv2".Document) as
*"github.com/google/gnostic-models/openapiv2".Document value in argument
to proto.NewOpenAPIData"
The error is due to pulling of a version of k8s.io/kube-openapi not
matching the one mentioned in the go.mod of client-go@v0.26.2
The fix is replacing the k8s.io/kube-openapi module with the one
mentioned in the go.mod of client-go@v0.26.2 - https://github.com/kubernetes/client-go/blob/v0.26.2/go.mod#L30C22-L30C56

The command to replace the module:

go mod edit -replace k8s.io/kube-openapi=k8s.io/kube-openapi@v0.0.0-20221012153701-172d655c2280

4. k8s.io/kubernetes should not be used

Reproducing the text here:

To use Kubernetes code as a library in other applications, see the list
of published components -
https://github.com/kubernetes/kubernetes/blob/master/staging/README.md
Use of the k8s.io/kubernetes module or
k8s.io/kubernetes/... packages as libraries is not supported.

Ref: https://pkg.go.dev/k8s.io/kubernetes#section-readme

5. Update peerpodconfig-ctl dep to include support for per-node peerpod
   limit

Fixes: #KATA-2271

Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Copy link
Member

@gkurz gkurz left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @bpradipt !

@bpradipt bpradipt removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 22, 2023
Copy link
Contributor

@littlejawa littlejawa left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@bpradipt bpradipt merged commit 9f3c953 into openshift:devel Sep 26, 2023
@bpradipt bpradipt deleted the fix-2271 branch September 26, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants