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.

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

OCPBUGS-44961: Fix "Enforce the required-scc monitor test and validate usage of non-standard OCP SCCs" #29323

Merged
merged 2 commits into from
Dec 2, 2024
Merged

OCPBUGS-44961: Fix "Enforce the required-scc monitor test and validate usage of non-standard OCP SCCs" #29323

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6897e4b
monitortests: enforce required-scc monitor test for fixed namespaces
kramaranya Nov 25, 2024
46485ae
monitortests: update flaking namespaces and exclude managed services
kramaranya Nov 26, 2024
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
Prev Previous commit
monitortests: update flaking namespaces and exclude managed services
  • Loading branch information
@kramaranya
kramaranya committed Nov 26, 2024
commit 46485aed8b4ed298129b4ca41c146b1f24a67127
41 changes: 31 additions & 10 deletions pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/openshift/origin/pkg/monitor/monitorapi"
"github.com/openshift/origin/pkg/monitortestframework"
"github.com/openshift/origin/pkg/test/ginkgo/junitapi"
exutil "github.com/openshift/origin/test/extended/util"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"

Expand All @@ -36,6 +37,7 @@ var nonStandardSCCNamespaces = map[string]sets.Set[string]{
"machine-api-termination-handler": sets.New("openshift-machine-api"),
}

// namespacesWithPendingSCCPinning includes namespaces with workloads that have pending SCC pinning.
var namespacesWithPendingSCCPinning = sets.NewString(
"openshift-cluster-csi-drivers",
"openshift-cluster-version",
Expand All @@ -45,8 +47,28 @@ var namespacesWithPendingSCCPinning = sets.NewString(
"openshift-ingress-operator",
"openshift-insights",
"openshift-machine-api",
"openshift-marketplace",
"openshift-monitoring",
// run-level namespaces
"openshift-cloud-controller-manager",
"openshift-cloud-controller-manager-operator",
"openshift-cluster-api",
"openshift-cluster-machine-approver",
"openshift-dns",
"openshift-dns-operator",
"openshift-etcd",
"openshift-etcd-operator",
"openshift-kube-apiserver",
"openshift-kube-apiserver-operator",
"openshift-kube-controller-manager",
"openshift-kube-controller-manager-operator",
"openshift-kube-proxy",
"openshift-kube-scheduler",
"openshift-kube-scheduler-operator",
"openshift-multus",
"openshift-network-operator",
"openshift-ovn-kubernetes",
"openshift-sdn",
"openshift-storage",
)
Copy link
Member

@stbenjam stbenjam Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you include https://github.com/openshift/origin/blob/master/test/extended/util/managed_services.go#L8-L28 in this set?

.Union(ManagedServiceNamespaces)

ROSA managed service namespaces had also failed on this enforcement


// systemNamespaces includes namespaces that should be treated as flaking.
Expand Down Expand Up @@ -90,19 +112,18 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD

junits := []*junitapi.JUnitTestCase{}
for _, ns := range namespaces.Items {
// require that all workloads in openshift, kube-* or default namespaces must have the required-scc annotation
// skip managed service namespaces
if exutil.ManagedServiceNamespaces.Has(ns.Name) {
continue
}
Comment on lines +116 to +118
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stbenjam we have decided to skip scc pinning for the namespaces in ManagedServiceNamespaces. If you believe these namespaces should instead be treated as flaking, please let us know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI ^^^ Flaking means that the test might sometimes pass and sometimes fail. By treating these namespaces as flaking, we allow our tests to continue without blocking progress. So if we decide to treat them as flaking, those namespaces should be fixed and eventually removed from the flaking list.

Copy link
Member

Choose a reason for hiding this comment

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

Skipping them entirely sounds fine, thanks


// require that all workloads in openshift, kube-*, or default namespaces must have the required-scc annotation
// ignore openshift-must-gather-* namespaces which are generated dynamically
isPermanentOpenShiftNamespace := (ns.Name == "openshift" || strings.HasPrefix(ns.Name, "openshift-")) && !strings.HasPrefix(ns.Name, "openshift-must-gather-")
if !strings.HasPrefix(ns.Name, "kube-") && ns.Name != "default" && !isPermanentOpenShiftNamespace {
continue
}

// check if the namespace should be treated as flaking when failed
flakeWhenFailed := ns.Labels["openshift.io/run-level"] == "0" ||
ns.Labels["openshift.io/run-level"] == "1" ||
namespacesWithPendingSCCPinning.Has(ns.Name) ||
systemNamespaces.Has(ns.Name)

pods, err := w.kubeClient.CoreV1().Pods(ns.Name).List(ctx, metav1.ListOptions{})
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -168,8 +189,8 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
FailureOutput: &junitapi.FailureOutput{Output: failureMsg},
})

// add a successful test with the same name to cause a flake
if flakeWhenFailed {
// add a successful test with the same name to cause a flake if the namespace should be flaking
if namespacesWithPendingSCCPinning.Has(ns.Name) || systemNamespaces.Has(ns.Name) {
junits = append(junits,
&junitapi.JUnitTestCase{
Name: testName,
Expand Down