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

SANDBOX-559 K8s 1 27 member #551

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

ranakan19
Copy link
Contributor

@ranakan19 ranakan19 commented Mar 25, 2024

Updates k8s dependencies to 0.27 and controller-runtime to v0.15.
With the update to controller-runtime v0.15 (changes in release detailed here), this PR has following changes to combat the breaking changes:

Changes wrt - https://issues.redhat.com/browse/SANDBOX-559
DO NOT MERGE (All the PRs related to version updates need to be merged at once, but these PR are for early feedback so that there are not a lot of changes to review at once)

controllers/idler/idler_controller_test.go Outdated Show resolved Hide resolved
controllers/nstemplateset/cluster_resources.go Outdated Show resolved Hide resolved
controllers/useraccount/useraccount_controller_test.go Outdated Show resolved Hide resolved
controllers/useraccount/useraccount_controller_test.go Outdated Show resolved Hide resolved
controllers/useraccount/useraccount_controller_test.go Outdated Show resolved Hide resolved
controllers/useraccount/useraccount_controller_test.go Outdated Show resolved Hide resolved
controllers/useraccount/useraccount_controller_test.go Outdated Show resolved Hide resolved
controllers/useraccount/useraccount_controller_test.go Outdated Show resolved Hide resolved
controllers/useraccount/useraccount_controller_test.go Outdated Show resolved Hide resolved
@@ -51,7 +52,7 @@ func TestConsolePluginServer(t *testing.T) {
func waitForReady(t *testing.T) {
cl := http.Client{}

err := wait.Poll(DefaultRetryInterval, DefaultTimeout, func() (done bool, err error) {
err := wait.PollUntilContextTimeout(context.TODO(), DefaultRetryInterval, DefaultTimeout, false, func(ctx context.Context) (done bool, err error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Poll is being deprecated and the linter was complaining. However to fix this there was a typo in the function name that was suggested as replacement, which was later fixed.
PTAL here if the context is used correctly - I'm not a 100% sure.
For reference please check - kubernetes/apimachinery#153 and the fix here - https://github.com/kubernetes/kubernetes/pull/117143/files

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 83.44%. Comparing base (1baa7db) to head (8d817d7).
Report is 1 commits behind head on master.

Files Patch % Lines
...trollers/nstemplateset/nstemplateset_controller.go 0.00% 4 Missing ⚠️
controllers/useraccount/useraccount_controller.go 0.00% 3 Missing ⚠️
...roperatorconfig/memberoperatorconfig_controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #551      +/-   ##
==========================================
- Coverage   83.56%   83.44%   -0.12%     
==========================================
  Files          28       28              
  Lines        2592     2592              
==========================================
- Hits         2166     2163       -3     
- Misses        287      289       +2     
- Partials      139      140       +1     
Files Coverage Δ
controllers/memberoperatorconfig/mapper.go 100.00% <100.00%> (ø)
...roperatorconfig/memberoperatorconfig_controller.go 75.00% <0.00%> (ø)
controllers/useraccount/useraccount_controller.go 84.13% <0.00%> (ø)
...trollers/nstemplateset/nstemplateset_controller.go 70.96% <0.00%> (-2.42%) ⬇️

Copy link

sonarcloud bot commented Apr 15, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
10.7% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@MatousJobanek MatousJobanek left a comment

Choose a reason for hiding this comment

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

Looks good, but I have a few comments on the versions in go.mod file.

Also, is there a reason why you picked the v0.27.2 specifically, and not the latest v0.27.x patch version that is available at the time when we are doing the update? This is just a detail since we are going to bump the version to the next minor one soon again.

go.mod Outdated
k8s.io/klog v1.0.0
k8s.io/klog/v2 v2.70.1
k8s.io/klog/v2 v2.90.1
k8s.io/metrics v0.25.0
Copy link
Contributor

Choose a reason for hiding this comment

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

why isn't this dependency k8s.io/metrics updated to v0.27.2 as well?

k8s.io/apimachinery v0.25.0
github.com/prometheus/client_golang v1.15.1
k8s.io/apiextensions-apiserver v0.27.2
k8s.io/apimachinery v0.27.2
k8s.io/kubectl v0.24.0
Copy link
Contributor

Choose a reason for hiding this comment

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

the same - it should use

Suggested change
k8s.io/kubectl v0.24.0
k8s.io/kubectl v0.27.2

go.mod Outdated
@@ -3,77 +3,67 @@ module github.com/codeready-toolchain/member-operator
require (
github.com/codeready-toolchain/api v0.0.0-20240322110702-5ab3840476e9
github.com/codeready-toolchain/toolchain-common v0.0.0-20240404090512-046d250d7d78
github.com/go-logr/logr v1.2.3
github.com/go-logr/logr v1.2.4
github.com/google/go-cmp v0.5.9
// using latest commit from 'github.com/openshift/api branch release-4.12'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// using latest commit from 'github.com/openshift/api branch release-4.12'
// using latest commit from 'github.com/openshift/api branch release-4.14'

github.com/google/go-cmp v0.5.9
// using latest commit from 'github.com/openshift/api branch release-4.12'
github.com/openshift/api v0.0.0-20230213134911-7ba313770556
github.com/openshift/api v0.0.0-20231117205818-971e4ba78c9a
github.com/pkg/errors v0.9.1
github.com/redhat-cop/operator-utils v1.3.3-0.20220121120056-862ef22b8cdf
Copy link
Contributor

Choose a reason for hiding this comment

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

let's update this dependency as well to match the version of k8s v.0.27.x - it should be the version v1.3.6 https://github.com/redhat-cop/operator-utils/releases/tag/v1.3.6

Copy link

openshift-ci bot commented Aug 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, ranakan19

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:
  • OWNERS [alexeykazakov,ranakan19]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

sonarcloud bot commented Aug 21, 2024

Copy link

openshift-ci bot commented Aug 21, 2024

@ranakan19: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e dbb501e link true /test e2e

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants