Skip to content

OCPEDGE-2303: update test logic for degraded cluster run #30649

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.

Sign up for GitHub

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

Open
Neilhamza wants to merge 3 commits into
base: main
Choose a base branch
from
+49 −17
Open

OCPEDGE-2303: update test logic for degraded cluster run #30649

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66455e1
update test logic for degraded tests
Neilhamza Jan 5, 2026
3717fab
update test to keep logic as-is
Neilhamza Jan 8, 2026
657fd3a
add env var gate for degraded mode
Neilhamza Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 35 additions & 10 deletions test/extended/operators/certs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/openshift/library-go/pkg/certs/cert-inspection/certgraphutils"
"github.com/openshift/library-go/pkg/operator/resource/resourceread"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/origin/pkg/certs"
"github.com/openshift/origin/pkg/monitortestlibrary/platformidentification"
testresult "github.com/openshift/origin/pkg/test/ginkgo/result"
Expand Down Expand Up @@ -128,8 +129,17 @@ var _ = g.Describe(fmt.Sprintf("[sig-arch][Late][Jira:%q]", "kube-apiserver"), g
// Skip metal jobs if test image pullspec cannot be determined
if jobType.Platform != "metal" || err == nil {
o.Expect(err).NotTo(o.HaveOccurred())
onDiskPKIContent, err = fetchOnDiskCertificates(ctx, kubeClient, oc.AdminConfig(), masters, openshiftTestImagePullSpec)
Copy link
Contributor

@xueqzhan xueqzhan Jan 6, 2026

Choose a reason for hiding this comment

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

So the presence of unready masters on line 124 does not cause any issue?

Copy link
Contributor Author

@Neilhamza Neilhamza Jan 6, 2026

Choose a reason for hiding this comment

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

i wondered that aswell! turns out The fact that one master is NotReady shouldn’t affect gatherCertsFromPlatformNamespaces in line 124. It collects all its data from the cluster API and uses the masters slice just for name-rewriting and cleanup. Unlike the on-disk collection path, it doesn’t pin any helper pods to nodes or reach out to kubelets.

o.Expect(err).NotTo(o.HaveOccurred())
topo, topoErr := exutil.GetControlPlaneTopology(oc)
o.Expect(topoErr).NotTo(o.HaveOccurred())

if *topo == configv1.DualReplicaTopologyMode {
Copy link
Contributor

@jaypoulz jaypoulz Jan 8, 2026

Choose a reason for hiding this comment

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

I feel like we should be able to refine this even further. Maybe what we need here is to add a flag to openshift-tests that allows us to specificy that we are running the suite in degraded mode. We know we're in degraded mode because the test pod has an env var set. What we'd need to do is to change the invocation of the test suite so that we check for that env var and pass it to the test suite as extra configuration/context. Then this test could make an except on our topology only when that configuration is set.

Copy link
Contributor Author

@Neilhamza Neilhamza Jan 8, 2026

Choose a reason for hiding this comment

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

pushed changes

readyMasters, _ := filterReadyNodes(masters)
onDiskPKIContent, err = fetchOnDiskCertificates(ctx, kubeClient, oc.AdminConfig(), readyMasters, openshiftTestImagePullSpec)
o.Expect(err).NotTo(o.HaveOccurred())
} else {
onDiskPKIContent, err = fetchOnDiskCertificates(ctx, kubeClient, oc.AdminConfig(), masters, openshiftTestImagePullSpec)
o.Expect(err).NotTo(o.HaveOccurred())
}
}

actualPKIContent = certgraphanalysis.MergePKILists(ctx, inClusterPKIContent, onDiskPKIContent)
Expand Down Expand Up @@ -160,14 +170,13 @@ var _ = g.Describe(fmt.Sprintf("[sig-arch][Late][Jira:%q]", "kube-apiserver"), g
o.Expect(err).NotTo(o.HaveOccurred())

pkiDir := filepath.Join(exutil.ArtifactDirPath(), "rawTLSInfo")
err = os.MkdirAll(pkiDir, 0755)
err = os.MkdirAll(pkiDir, 0o755)
o.Expect(err).NotTo(o.HaveOccurred())
err = os.WriteFile(filepath.Join(pkiDir, tlsArtifactFilename), jsonBytes, 0644)
err = os.WriteFile(filepath.Join(pkiDir, tlsArtifactFilename), jsonBytes, 0o644)
o.Expect(err).NotTo(o.HaveOccurred())
})

g.It("all tls artifacts must be registered", func() {

violationsPKIContent, err := certs.GetPKIInfoFromEmbeddedOwnership(ownership.PKIViolations)
o.Expect(err).NotTo(o.HaveOccurred())

Expand All @@ -181,7 +190,6 @@ var _ = g.Describe(fmt.Sprintf("[sig-arch][Late][Jira:%q]", "kube-apiserver"), g

_, err := certgraphutils.LocateCertKeyPairBySecretLocation(currLocation, expectedPKIContent.CertKeyPairs)
if err != nil {

newTLSRegistry.CertKeyPairs = append(newTLSRegistry.CertKeyPairs, certgraphapi.PKIRegistryCertKeyPair{InClusterLocation: &actualPKIContent.InClusterResourceData.CertKeyPairs[i]})
}

Expand Down Expand Up @@ -269,11 +277,11 @@ var _ = g.Describe(fmt.Sprintf("[sig-arch][Late][Jira:%q]", "kube-apiserver"), g
if len(newTLSRegistry.CertKeyPairs) > 0 || len(newTLSRegistry.CertificateAuthorityBundles) > 0 {
registryString, err := json.MarshalIndent(newTLSRegistry, "", " ")
if err != nil {
//g.Fail("Failed to marshal registry %#v: %v", newTLSRegistry, err)
// g.Fail("Failed to marshal registry %#v: %v", newTLSRegistry, err)
testresult.Flakef("Failed to marshal registry %#v: %v", newTLSRegistry, err)
}
// TODO: uncomment when test no longer fails and enhancement is merged
//g.Fail(fmt.Sprintf("Unregistered TLS certificates:\n%s", registryString))
// g.Fail(fmt.Sprintf("Unregistered TLS certificates:\n%s", registryString))
testresult.Flakef("Unregistered TLS certificates found:\n%s\nSee tls/ownership/README.md in origin repo", registryString)
}
})
Expand All @@ -285,7 +293,7 @@ var _ = g.Describe(fmt.Sprintf("[sig-arch][Late][Jira:%q]", "kube-apiserver"), g

if len(messages) > 0 {
// TODO: uncomment when test no longer fails and enhancement is merged
//g.Fail(strings.Join(messages, "\n"))
// g.Fail(strings.Join(messages, "\n"))
testresult.Flakef("%s", strings.Join(messages, "\n"))
}
})
Expand Down Expand Up @@ -323,7 +331,6 @@ var _ = g.Describe(fmt.Sprintf("[sig-arch][Late][Jira:%q]", "kube-apiserver"), g
testresult.Flakef("Errors found: %s", utilerrors.NewAggregate(errs).Error())
}
})

})

func fetchOnDiskCertificates(ctx context.Context, kubeClient kubernetes.Interface, podRESTConfig *rest.Config, nodeList []*corev1.Node, testPullSpec string) (*certgraphapi.PKIList, error) {
Expand Down Expand Up @@ -480,3 +487,21 @@ func isCertKeyPairFromIgnoredNamespace(cert certgraphapi.CertKeyPair, ignoredNam
}
return false
}

func filterReadyNodes(nodes []*corev1.Node) (ready []*corev1.Node, notReady []string) {
for _, n := range nodes {
isReady := false
for _, c := range n.Status.Conditions {
if c.Type == corev1.NodeReady && c.Status == corev1.ConditionTrue {
isReady = true
break
}
}
if isReady {
ready = append(ready, n)
} else {
notReady = append(notReady, n.Name)
}
}
return ready, notReady
}