Skip to content

Conversation

rsoaresd
Copy link
Contributor

@rsoaresd rsoaresd commented Apr 23, 2025

Description

Update dependencies

Tool/Library Current Version Updates to Version
Operator SDK 1.38 1.39.2
k8s.io/* v0.30.1 v0.31.2
controller-runtime v0.18.4 v0.19.7
controller-tools v0.15.0 v0.16.5

Issue ticket number and link

SANDBOX-955

cmd/main.go Outdated
@@ -178,6 +180,9 @@ func main() {
// disable caching of Node metrics in the client to avoid getting the following error every second
// "failed to watch *v1beta1.NodeMetrics: the server does not allow this method on the requested resource (get nodes.metrics.k8s.io)"
Client: client.Options{Cache: &client.CacheOptions{DisableFor: []client.Object{&kmetrics.NodeMetrics{}}}},
Controller: ctrlconfig.Controller{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to this change in sigs.k8s.io/controller-runtime (introduced in v0.19.0), we need to skip the name validation to avoid this error:

{"level":"error","ts":"2025-04-22T10:46:41.577Z","logger":"setup","msg":"unable to create controller","controller":"ToolchainCluster","error":"controller with name toolchaincluster already exists. Controller names must be unique to avoid multiple controllers reporting to the same metric","stacktrace":"main.main\n\t/Users/rsoaresd/member-operator/cmd/main.go:245\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:271"}

Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, is there a way to configure the controller name?

Copy link
Contributor

Choose a reason for hiding this comment

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

I found it - there is Named function in the controller builder. We could do something like this instead

func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
	return ctrl.NewControllerManagedBy(mgr).
		Named("ToolchainClusterCache").
          ...

and the same for other duplicated controllers too - that should be pretty simple change, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to test!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried changing in controllers/idler/idler_controller.go:

// SetupWithManager sets up the controller with the Manager.
func (r *Reconciler) SetupWithManager(mgr manager.Manager, allNamespaceCluster runtimeCluster.Cluster) error {
	return ctrl.NewControllerManagedBy(mgr).
		Named("ToolchainClusterCache").
		For(&toolchainv1alpha1.Idler{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
		WatchesRawSource(source.Kind(allNamespaceCluster.GetCache(), &corev1.Pod{},
			handler.TypedEnqueueRequestsFromMapFunc(MapPodToIdler), PodIdlerPredicate{})).
		Complete(r)
}

But it reports:
{"level":"error","ts":"2025-04-23T13:25:23.269Z","logger":"setup","msg":"unable to create controller","controller":"ToolchainCluster","error":"controller with name toolchaincluster already exists. Controller names must be unique to avoid multiple controllers reporting to the same metric","stacktrace":"main.main\n\t/Users/rsoaresd/member-operator/cmd/main.go:245\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:271"}

Copy link
Contributor

Choose a reason for hiding this comment

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

But you should update the toolchaincluster_cache_controller.go in toolchain-common. The idler controller doesn't watch ToolchainCluster CRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you so much, addressed it! codeready-toolchain/toolchain-common@9d0638b

Copy link

codecov bot commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.09%. Comparing base (4555688) to head (ddbb573).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #665   +/-   ##
=======================================
  Coverage   81.09%   81.09%           
=======================================
  Files          46       46           
  Lines        4506     4506           
=======================================
  Hits         3654     3654           
  Misses        690      690           
  Partials      162      162           
Files with missing lines Coverage Δ
controllers/useraccount/console_usersettings.go 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

openshift-ci bot commented May 6, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, MatousJobanek, rsoaresd

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 [MatousJobanek,alexeykazakov,rsoaresd]

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

Copy link

sonarqubecloud bot commented May 6, 2025

@rsoaresd rsoaresd merged commit 8911b4f into codeready-toolchain:master May 6, 2025
13 of 15 checks passed
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