Skip to content

fix : remove kube-rbac-proxy sidecar metric proxy container from Operator deployment - #1437

Merged
dkwon17 merged 2 commits into
devfile:mainfrom
rohankanojia-forks:pr/issue1343
Jul 9, 2025
Merged

fix : remove kube-rbac-proxy sidecar metric proxy container from Operator deployment#1437
dkwon17 merged 2 commits into
devfile:mainfrom
rohankanojia-forks:pr/issue1343

Conversation

@rohanKanojia

@rohanKanojiarohanKanojia commented May 22, 2025

Copy link
Copy Markdown
Member

What does this PR do?

As mentioned in #1352 and #1343 (comment) , using kube-rbac-proxy sidecar container is deprecated and is causing issues in configuring Operator resource/limits via Operator subscription.

This PR removes this sidecar container and uses controller-runtime's inbuilt WithAuthenticationAndAuthorization. Now that we don't have a proxy, we can directly post metrics on 8443 and 9443 ports for devworkspace-controller-manager and devworkspace-webhook-server respectively.

Signed-off-by: Rohan Kumar rohaan@redhat.com

What issues does this PR fix or reference?

#1343

Is it tested? How?

After making changes I made sure operator is running and metrics are accessible on 8443 and 9443 ports.

  • setup OpenShift cluster
  • Install DWO from this PR:
oc apply -f - <<EOFapiVersion: operators.coreos.com/v1alpha1kind: CatalogSourcemetadata: name: devworkspace-operator-catalog namespace: openshift-marketplacespec: sourceType: grpc image: docker.io/rohankanojia/devworkspace-operator-index:next publisher: Red Hat displayName: DevWorkspace Operator Catalog updateStrategy: registryPoll: interval: 5mEOF
oc apply -f - <<EOFapiVersion: operators.coreos.com/v1alpha1kind: Subscriptionmetadata: name: devworkspace-operator namespace: openshift-operatorsspec: channel: next installPlanApproval: Automatic name: devworkspace-operator source: devworkspace-operator-catalog sourceNamespace: openshift-marketplaceEOF
  • Check operator pods, they'd have only one running container (previously all pods have 2 containers each)
    devworkspace-operator : $ oc get pods
    NAME READY STATUS RESTARTS AGE
    devworkspace-controller-manager-56679dd464-lnh2g 1/1 Running 0 15m
    devworkspace-webhook-server-5874545c5c-58n4s 1/1 Running 0 15m
    devworkspace-webhook-server-5874545c5c-ntfmt 1/1 Running 1 (15m ago) 15m
    
  • Port forward metrics service and try to access metrics
    kubectl port-forward service/devworkspace-controller-metrics 8443:8443 &
    kubectl port-forward service/devworkspace-webhookserver 9443:9443
    # [In separate session]
    NAMESPACE=devworkspace-controller kubectl create clusterrolebinding dw-metrics \
    --clusterrole=devworkspace-controller-metrics-reader \
    --serviceaccount=${NAMESPACE}:devworkspace-controller-serviceaccount
    TOKEN=$(kubectl create token devworkspace-controller-serviceaccount -n ${NAMESPACE} --duration=1h)
    curl -k -H "Authorization: Bearer ${TOKEN}" https://localhost:9443/metrics
    curl -k -H "Authorization: Bearer ${TOKEN}" https://localhost:8443/metrics

Configuring memory and CPU requests/limits

  • Patch the existing Subscription to have CPU requests and limits
kubectl patch subscription devworkspace-operator \
-n openshift-operators \
--type merge \
-p '{ "spec": { "config": { "resources": { "requests": { "cpu": "100m", "memory": "256Mi" }, "limits": { "cpu": "500m", "memory": "512Mi" } } } } }'
  • After applying abovementioned changes, check state of DevWorkspace Operator Pods to see if memory requests/limits have been added or not:
kubectl get pods -l app.kubernetes.io/name=devworkspace-controller -o custom-columns=NAME:.metadata.name,CPU_REQ:.spec.containers[*].resources.requests.cpu,CPU_LIM:.spec.containers[*].resources.limits.cpu,MEM_REQ:.spec.containers[*].resources.requests.memory,MEM_LIM:.spec.containers[*].resources.limits.memory
NAME CPU_REQ CPU_LIM MEM_REQ MEM_LIM
devworkspace-controller-manager-6ddc89c564-4fxwt 100m 500m 256Mi 512Mi

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

@openshift-ci

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rohanKanojia
rohanKanojiaforce-pushed the pr/issue1343 branch 3 times, most recently from 2a64641 to bc2d72cCompareMay 22, 2025 15:12
@rohanKanojia

Copy link
Copy Markdown
MemberAuthor

/ok-to-test

@rohanKanojia
rohanKanojiaforce-pushed the pr/issue1343 branch 2 times, most recently from 4c4e17e to 5e719a8CompareMay 26, 2025 10:06
@rohanKanojia

Copy link
Copy Markdown
MemberAuthor

/ok-to-test

@rohanKanojia

Copy link
Copy Markdown
MemberAuthor

/ok-to-test

1 similar comment
@rohanKanojia

Copy link
Copy Markdown
MemberAuthor

/ok-to-test

@rohanKanojia
rohanKanojia marked this pull request as ready for review July 1, 2025 05:46
@openshift-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dkwon17, rohanKanojia

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

Comment threadmain.go
Metrics: metricsserver.Options{
BindAddress: metricsAddr,
BindAddress: metricsAddr,
FilterProvider: filters.WithAuthenticationAndAuthorization,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since kube-rbac-proxy was configured to use HTTPS with --secure-listen-address=0.0.0.0:8443, could we also set the metrics server to use HTTPS?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Basically, this documentation should work the same way as before: https://eclipse.dev/che/docs/stable/administration-guide/monitoring-the-dev-workspace-operator/

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Good catch, I've added SecureServing: true in metricsserver.Options. It enables https in metrics server

@openshift-ciopenshift-ciBot removed the lgtm label Jul 4, 2025
@openshift-ci

Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@dkwon17
dkwon17 merged commit 608bf21 into devfile:mainJul 9, 2025
@rohanKanojia
rohanKanojia deleted the pr/issue1343 branch July 9, 2025 13:03
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@rohanKanojia@dkwon17@openshift-merge-robot