OCPBUGS-54238: Update CSR status condition appropriately #2674

pperiyasamy · 2025-03-31T11:45:50Z

When CSR signing reattempt happens, signer controller is not updating existing CertificateFailed condition type, instead it tries to add another CertificateFailed condition and leads to Duplicate value: "Failed" error in the network co status,

 % oc get co network
NAME      VERSION                                                   AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
network   4.19.0-0.ci.test-2025-03-26-015315-ci-ln-g8dqch2-latest   True        False         True       4h      Unable to update csr: CertificateSigningRequest.certificates.k8s.io "ipsec-csr-test-80237" is invalid: status.conditions[1].type: Duplicate value: "Failed"

Fixing it just by updating existing CertificateFailed condition so that network status don't get updated unnecessarily for this case.

openshift-ci-robot · 2025-03-31T11:45:58Z

@pperiyasamy: This pull request references Jira Issue OCPBUGS-54238, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.19.0) matches configured target version for branch (4.19.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @huiran0826

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

The signer controller is reflecting CSR approval status into network operator status during signing process for every CSR, when CSR is removed it's not getting network status back into original state because CSR is no longer available.

When CSR signing reattempt happens upon a failure, signer controller is not updating existing CertificateFailed condition type, instead it tries to add another CertificateFailed condition and leads to below error:
% oc get co network
NAME      VERSION                                                   AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
network   4.19.0-0.ci.test-2025-03-26-015315-ci-ln-g8dqch2-latest   True        False         True       4h      Unable to update csr: CertificateSigningRequest.certificates.k8s.io "ipsec-csr-test-80237" is invalid: status.conditions[1].type: Duplicate value: "Failed"
so fixing it by updating existing CertificateFailed condition.

Depends on #2560.

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 openshift-eng/jira-lifecycle-plugin repository.

pperiyasamy · 2025-04-02T12:52:00Z

/retest

pperiyasamy · 2025-04-02T12:52:14Z

/assign @martinkennelly @trozet

martinkennelly

/lgtm

kyrtapz · 2025-04-17T15:39:34Z

pkg/controller/signer/signer-controller.go

@@ -88,6 +89,8 @@ func (r *ReconcileCSR) Reconcile(ctx context.Context, request reconcile.Request)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			// Request object not found, could have been deleted after reconcile request.
+			// restore network status when CSR is deleted.
+			r.status.SetNotDegraded(statusmanager.CertificateSigner)


What if there is more than one CSR that is failing and one of those gets removed?
Was it always broken like that?

yes @kyrtapz, the status gets updated again when next signing attempt happens for another CSR.

I still don't see this as the way to go, what if a CSR signing fails for one object but another CSR expired in the meantime and got garbage collected? We would remove the error.
To me this changes how CNO behaved until now. There is a wider issue of the status being overwritten for every CSR but this is a different discussion.
Is this change still necessary with the other fix you did?

To summarize, I don't think we should change the CNO degraded state based on any particular CSR object, this should be reserved only for internal controller failures.

kyrtapz · 2025-04-17T15:43:39Z

pkg/controller/signer/signer-controller.go

@@ -251,7 +254,7 @@ func signerFailure(r *ReconcileCSR, csr *csrv1.CertificateSigningRequest, reason

 // Update the status conditions on the CSR object
 func updateCSRStatusConditions(r *ReconcileCSR, csr *csrv1.CertificateSigningRequest, reason string, message string) {
-	csr.Status.Conditions = append(csr.Status.Conditions, csrv1.CertificateSigningRequestCondition{
+	setCertificateSigningRequestCondition(&csr.Status.Conditions, csrv1.CertificateSigningRequestCondition{


Why not use this?

cluster-network-operator/vendor/k8s.io/apimachinery/pkg/api/meta/conditions.go

Line 31 in 621fb76

func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Condition) (changed bool) {

this is updating k8s.io/api/certificates/v1.CertificateSigningRequestCondition object here, so not using generic Condition object.

kyrtapz · 2025-04-17T16:17:22Z

@pperiyasamy what was the CSR that caused the issue? We should not retry denied CSRs

kyrtapz · 2025-04-17T16:37:51Z

pkg/controller/signer/signer-controller.go

+		existingCondition.Status = newCondition.Status
+		existingCondition.LastTransitionTime = metav1.NewTime(time.Now())
+	}
+	existingCondition.Reason = newCondition.Reason


Why aren't you updating the LastTransitionTime if the Status doesn't change?

sure, updated it.

When CSR signing reattempt happens, signer controller is not updating existing CertificateFailed condition type, instead it tries to add another CertificateFailed condition and leads to Duplicate value: "Failed" error in the network co status, so fixing it just by updating existing CertificateFailed condition. Signed-off-by: Periyasamy Palanisamy <[email protected]>

openshift-ci-robot · 2025-04-17T17:30:52Z

@pperiyasamy: This pull request references Jira Issue OCPBUGS-54238, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.19.0) matches configured target version for branch (4.19.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @huiran0826

In response to this:

When CSR signing reattempt happens, signer controller is not updating existing CertificateFailed condition type, instead it tries to add another CertificateFailed condition and leads to Duplicate value: "Failed" error in the network co status,
% oc get co network
NAME      VERSION                                                   AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
network   4.19.0-0.ci.test-2025-03-26-015315-ci-ln-g8dqch2-latest   True        False         True       4h      Unable to update csr: CertificateSigningRequest.certificates.k8s.io "ipsec-csr-test-80237" is invalid: status.conditions[1].type: Duplicate value: "Failed"
Fixing it just by updating existing CertificateFailed condition so that network status don't get updated unnecessarily for this case.

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 openshift-eng/jira-lifecycle-plugin repository.

kyrtapz · 2025-04-17T17:41:25Z

/lgtm

openshift-ci · 2025-04-17T17:42:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyrtapz, martinkennelly, pperiyasamy

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~~ [kyrtapz]

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

openshift-ci-robot · 2025-04-17T20:23:09Z

/retest-required

Remaining retests: 0 against base HEAD 50405c0 and 2 for PR HEAD a33bca4 in total

zshi-redhat · 2025-04-18T03:36:28Z

/retest-required

ricky-rav · 2025-04-18T08:00:30Z

/retest-required

openshift-ci-robot · 2025-04-18T10:22:26Z

/retest-required

Remaining retests: 0 against base HEAD 50405c0 and 2 for PR HEAD a33bca4 in total

ricky-rav · 2025-04-18T12:23:53Z

/retest-required

ricky-rav · 2025-04-18T13:44:20Z

/retest-required

openshift-ci-robot · 2025-04-18T19:22:26Z

/retest-required

Remaining retests: 0 against base HEAD b0aaa7d and 1 for PR HEAD a33bca4 in total

zshi-redhat · 2025-04-21T03:32:28Z

/retest-required

openshift-ci-robot · 2025-04-22T10:20:32Z

/retest-required

Remaining retests: 0 against base HEAD b0aaa7d and 2 for PR HEAD a33bca4 in total

openshift-ci-robot · 2025-04-22T14:25:10Z

/retest-required

Remaining retests: 0 against base HEAD 6268c4e and 1 for PR HEAD a33bca4 in total

openshift-ci-robot · 2025-04-22T20:28:39Z

/retest-required

Remaining retests: 0 against base HEAD 6268c4e and 2 for PR HEAD a33bca4 in total

pperiyasamy · 2025-04-23T09:38:41Z

/retest-required

jcaamano · 2025-04-23T11:17:29Z

/override ci/prow/e2e-aws-ovn-windows

perma-failing due to https://issues.redhat.com/browse/WINC-1384

jcaamano · 2025-04-23T11:21:37Z

/override ci/prow/e2e-aws-ovn-ipsec-upgrade

no related to this change
https://issues.redhat.com/browse/OCPBUGS-55262

openshift-ci · 2025-04-23T11:22:32Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/e2e-aws-ovn-windows

In response to this:

/override ci/prow/e2e-aws-ovn-windows

perma-failing due to https://issues.redhat.com/browse/WINC-1384

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.

openshift-ci · 2025-04-23T11:24:10Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/e2e-aws-ovn-ipsec-upgrade

In response to this:

/override ci/prow/e2e-aws-ovn-ipsec-upgrade

no related to this change
https://issues.redhat.com/browse/OCPBUGS-55262

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.

openshift-ci-robot · 2025-04-23T12:33:28Z

/retest-required

Remaining retests: 0 against base HEAD bcf7b32 and 1 for PR HEAD a33bca4 in total

jcaamano · 2025-04-23T14:41:33Z

/override ci/prow/e2e-aws-ovn-windows
/override ci/prow/e2e-aws-ovn-ipsec-upgrade

openshift-ci · 2025-04-23T14:41:51Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/e2e-aws-ovn-ipsec-upgrade, ci/prow/e2e-aws-ovn-windows

In response to this:

/override ci/prow/e2e-aws-ovn-windows
/override ci/prow/e2e-aws-ovn-ipsec-upgrade

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.

jcaamano · 2025-04-23T16:10:53Z

/override ci/prow/e2e-aws-ovn-windows

openshift-ci · 2025-04-23T16:11:40Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/e2e-aws-ovn-windows

In response to this:

/override ci/prow/e2e-aws-ovn-windows

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.

jcaamano · 2025-04-23T16:13:59Z

/override ci/prow/e2e-aws-ovn-windows

This has passed before and last iteration failed during deprovision due to overall job timeout

{"component":"entrypoint","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:169","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.ExecuteProcess","level":"error","msg":"Process did not finish before 4h0m0s timeout","severity":"error","time":"2025-04-23T15:29:31Z"}
INFO[2025-04-23T15:29:31Z] Received signal.                              signal=interrupt
...
ERRO[2025-04-23T15:30:55Z] Some steps failed:                           
ERRO[2025-04-23T15:30:55Z] 
  * could not run steps: execution cancelled 
INFO[2025-04-23T15:30:55Z] Reporting job state 'failed' with reason 'executing_graph:interrupted' 
{"component":"entrypoint","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:264","func":"sigs.k8s.io/prow/pkg/entrypoint.gracefullyTerminate","level":"error","msg":"Process gracefully exited before 1h0m0s grace period","severity":"error","time":"2025-04-23T15:30:55Z"}

openshift-ci · 2025-04-23T16:14:36Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/e2e-aws-ovn-windows

In response to this:

/override ci/prow/e2e-aws-ovn-windows

This has passed before and last iteration failed during deprovision due to overall job timeout

{"component":"entrypoint","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:169","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.ExecuteProcess","level":"error","msg":"Process did not finish before 4h0m0s timeout","severity":"error","time":"2025-04-23T15:29:31Z"}
INFO[2025-04-23T15:29:31Z] Received signal.                              signal=interrupt
...
ERRO[2025-04-23T15:30:55Z] Some steps failed:                           
ERRO[2025-04-23T15:30:55Z] 
 * could not run steps: execution cancelled 
INFO[2025-04-23T15:30:55Z] Reporting job state 'failed' with reason 'executing_graph:interrupted' 
{"component":"entrypoint","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:264","func":"sigs.k8s.io/prow/pkg/entrypoint.gracefullyTerminate","level":"error","msg":"Process gracefully exited before 1h0m0s grace period","severity":"error","time":"2025-04-23T15:30:55Z"}

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.

jcaamano · 2025-04-23T16:15:27Z

/override ci/prow/e2e-aws-ovn-upgrade

last override was meant to be this one

openshift-ci · 2025-04-23T16:15:47Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/e2e-aws-ovn-upgrade

In response to this:

/override ci/prow/e2e-aws-ovn-upgrade

last override was meant to be this one

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.

openshift-ci · 2025-04-23T16:15:49Z

@pperiyasamy: The following tests 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/4.19-upgrade-from-stable-4.18-e2e-azure-ovn-upgrade	`a33bca4`	link	false	`/test 4.19-upgrade-from-stable-4.18-e2e-azure-ovn-upgrade`
ci/prow/e2e-ovn-hybrid-step-registry	`a33bca4`	link	false	`/test e2e-ovn-hybrid-step-registry`
ci/prow/e2e-aws-ovn-serial	`a33bca4`	link	false	`/test e2e-aws-ovn-serial`
ci/prow/4.19-upgrade-from-stable-4.18-e2e-gcp-ovn-upgrade	`a33bca4`	link	false	`/test 4.19-upgrade-from-stable-4.18-e2e-gcp-ovn-upgrade`
ci/prow/e2e-vsphere-ovn-dualstack-primaryv6	`a33bca4`	link	false	`/test e2e-vsphere-ovn-dualstack-primaryv6`
ci/prow/security	`a33bca4`	link	false	`/test security`
ci/prow/e2e-aws-ovn-shared-to-local-gateway-mode-migration	`a33bca4`	link	false	`/test e2e-aws-ovn-shared-to-local-gateway-mode-migration`
ci/prow/e2e-aws-hypershift-ovn-kubevirt	`a33bca4`	link	false	`/test e2e-aws-hypershift-ovn-kubevirt`
ci/prow/e2e-aws-ovn-local-to-shared-gateway-mode-migration	`a33bca4`	link	false	`/test e2e-aws-ovn-local-to-shared-gateway-mode-migration`
ci/prow/okd-scos-e2e-aws-ovn	`a33bca4`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/4.19-upgrade-from-stable-4.18-e2e-aws-ovn-upgrade	`a33bca4`	link	false	`/test 4.19-upgrade-from-stable-4.18-e2e-aws-ovn-upgrade`
ci/prow/e2e-ovn-step-registry	`a33bca4`	link	false	`/test e2e-ovn-step-registry`
ci/prow/e2e-aws-ovn-single-node	`a33bca4`	link	false	`/test e2e-aws-ovn-single-node`

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.

jcaamano · 2025-04-23T16:18:48Z

/override ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec

Has passed before, unrelated, failing on other PRs, tracked in https://issues.redhat.com/browse/OCPBUGS-55280

openshift-ci · 2025-04-23T16:19:06Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec

Has passed before, unrelated, failing on other PRs, tracked in https://issues.redhat.com/browse/OCPBUGS-55280

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.

openshift-ci-robot · 2025-04-23T16:24:23Z

@pperiyasamy: Jira Issue OCPBUGS-54238: All pull requests linked via external trackers have merged:

openshift/cluster-network-operator#2674

Jira Issue OCPBUGS-54238 has been moved to the MODIFIED state.

In response to this:

When CSR signing reattempt happens, signer controller is not updating existing CertificateFailed condition type, instead it tries to add another CertificateFailed condition and leads to Duplicate value: "Failed" error in the network co status,
% oc get co network
NAME      VERSION                                                   AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
network   4.19.0-0.ci.test-2025-03-26-015315-ci-ln-g8dqch2-latest   True        False         True       4h      Unable to update csr: CertificateSigningRequest.certificates.k8s.io "ipsec-csr-test-80237" is invalid: status.conditions[1].type: Duplicate value: "Failed"
Fixing it just by updating existing CertificateFailed condition so that network status don't get updated unnecessarily for this case.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-bot · 2025-04-23T21:07:47Z

[ART PR BUILD NOTIFIER]

Distgit: cluster-network-operator
This PR has been included in build cluster-network-operator-container-v4.19.0-202504232036.p0.ge6d5a38.assembly.stream.el9.
All builds following this will include this PR.

openshift-ci bot requested review from huiran0826, kyrtapz and tssurya March 31, 2025 11:46

openshift-ci bot assigned martinkennelly and trozet Apr 2, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2025

martinkennelly approved these changes Apr 15, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 15, 2025

pperiyasamy force-pushed the cert-signer-controller-status branch from bd8d812 to 749d72a Compare April 16, 2025 08:06

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2025

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 16, 2025

kyrtapz approved these changes Apr 17, 2025

View reviewed changes

kyrtapz reviewed Apr 17, 2025

View reviewed changes

pperiyasamy force-pushed the cert-signer-controller-status branch from 749d72a to a33bca4 Compare April 17, 2025 17:14

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 17, 2025

pperiyasamy changed the title ~~OCPBUGS-54238: Reset network status when CSR is deleted~~ OCPBUGS-54238: Update CSR status condition appropriately Apr 17, 2025

openshift-ci bot assigned kyrtapz Apr 17, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 17, 2025

openshift-merge-bot bot merged commit e6d5a38 into openshift:master Apr 23, 2025
24 of 37 checks passed

OCPBUGS-54238: Update CSR status condition appropriately #2674

OCPBUGS-54238: Update CSR status condition appropriately #2674

Uh oh!

Conversation

pperiyasamy commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 31, 2025

Uh oh!

pperiyasamy commented Apr 2, 2025

Uh oh!

pperiyasamy commented Apr 2, 2025

Uh oh!

martinkennelly left a comment

Choose a reason for hiding this comment

Uh oh!

kyrtapz Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pperiyasamy Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

kyrtapz Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyrtapz Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

kyrtapz Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

pperiyasamy Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

kyrtapz commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kyrtapz Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

pperiyasamy Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Apr 17, 2025

Uh oh!

kyrtapz commented Apr 17, 2025

Uh oh!

openshift-ci bot commented Apr 17, 2025

Uh oh!

openshift-ci-robot commented Apr 17, 2025

Uh oh!

zshi-redhat commented Apr 18, 2025

Uh oh!

ricky-rav commented Apr 18, 2025

Uh oh!

openshift-ci-robot commented Apr 18, 2025

Uh oh!

ricky-rav commented Apr 18, 2025

Uh oh!

ricky-rav commented Apr 18, 2025

Uh oh!

openshift-ci-robot commented Apr 18, 2025

Uh oh!

zshi-redhat commented Apr 21, 2025

Uh oh!

openshift-ci-robot commented Apr 22, 2025

Uh oh!

openshift-ci-robot commented Apr 22, 2025

Uh oh!

openshift-ci-robot commented Apr 22, 2025

Uh oh!

pperiyasamy commented Apr 23, 2025

Uh oh!

jcaamano commented Apr 23, 2025

Uh oh!

jcaamano commented Apr 23, 2025

Uh oh!

openshift-ci bot commented Apr 23, 2025

pperiyasamy commented Mar 31, 2025 •

edited

Loading

kyrtapz Apr 17, 2025 •

edited

Loading

kyrtapz Apr 17, 2025 •

edited

Loading

kyrtapz commented Apr 17, 2025 •

edited

Loading

openshift-ci bot commented Apr 23, 2025 •

edited

Loading