Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,15 @@
"source": "openshift:payload:cluster-version-operator",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[Jira:\"Cluster Version Operator\"] cluster-version-operator-tests with cluster access should have correct runlevel and scc",
"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:cluster-version-operator",
"lifecycle": "blocking",
"environmentSelector": {}
}
]
61 changes: 61 additions & 0 deletions test/cvo/cvo.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
package cvo

import (
"context"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
Expand All @@ -9,4 +17,57 @@ var _ = Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator-tes
It("should support passing tests", func() {
Expect(true).To(BeTrue())
})

Context("with cluster access", func() {
Copy link
Member

Choose a reason for hiding this comment

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

You may start another var _ = Describe() if you want so that the test specs do not have to start with cluster-version-operator-tests ....

I am think of something like "[Jira:"Cluster Version Operator"] cluster-version-operator should have right permissions"

Copy link
Author

Choose a reason for hiding this comment

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

If we accept two Describe blocks, sounds good to me to move Context to a new Describe.

const cvoNamespace = "openshift-cluster-version"
var (
restCfg *rest.Config
kubeClient kubernetes.Interface
)

BeforeEach(func() {
var err error

// Respects KUBECONFIG env var
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
cfg := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &clientcmd.ConfigOverrides{})
restCfg, err = cfg.ClientConfig()
Expect(err).NotTo(HaveOccurred(), "Failed to load Kubernetes configuration. Please ensure KUBECONFIG environment variable is set.")

kubeClient, err = kubernetes.NewForConfig(restCfg)
Expect(err).NotTo(HaveOccurred(), "Failed to create Kubernetes client")
Copy link
Member

Choose a reason for hiding this comment

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

I would make a util function such as GetKubeClient() (client, error). I am sure it will be used by other tests too.

Copy link
Author

Choose a reason for hiding this comment

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

I can move the client initialization into a utility function for potential future use, although I'm currently unsure if any other tests will need it since it's already added in BeforeEach.

})

// Migrated from case NonHyperShiftHOST-Author:jiajliu-Low-46922-check runlevel and scc in cvo ns
// Refer to https://github.com/openshift/openshift-tests-private/blob/master/test/extended/ota/cvo/cvo.go#L1081
Copy link
Member

Choose a reason for hiding this comment

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

Permalink is more stable.

Suggested change
// Refer to https://github.com/openshift/openshift-tests-private/blob/master/test/extended/ota/cvo/cvo.go#L1081
// Refer to https://github.com/openshift/openshift-tests-private/blob/40374cf20946ff03c88712839a5626af2c88ab31/test/extended/ota/cvo/cvo.go#L1081

It("should have correct runlevel and scc", func() {
Copy link
Member

Choose a reason for hiding this comment

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

I am thinking of doing the two cases directly without this level.

  • one for "CVO namespace has the run-level label" and
  • one for "CVO pod has the SCC annotation".

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, to ensure consistency during this case migration, I intend to keep the two checkpoints(By) within a single case(It). This will also allow for correct mapping to the existing cases in our Polarion case library.

ctx := context.Background()

// Skip the test on HyperShift since CVO runs in management cluster instead of hosted cluster
_, err := kubeClient.AppsV1().Deployments(cvoNamespace).Get(ctx, "cluster-version-operator", metav1.GetOptions{})
Copy link
Member

Choose a reason for hiding this comment

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

if errors.IsNotFound(err) {
Skip("Skipping test: CVO deployment not found in hypershift hosted cluster!")
}
Expect(err).NotTo(HaveOccurred(), "Failed to get CVO deployment")

By("Check runlevel in cvo namespace.")
ns, err := kubeClient.CoreV1().Namespaces().Get(ctx, cvoNamespace, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred(), "Failed to get namespace %s", cvoNamespace)

runLevel := ns.ObjectMeta.Labels["openshift.io/run-level"]
Copy link
Member

Choose a reason for hiding this comment

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

Which do you want to verify?
"Key exists and value is empty" or "key does not exist"?

Copy link
Author

Choose a reason for hiding this comment

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

the former, let me update it to be more clear

Expect(runLevel).To(Equal(""), "Expected 'openshift.io/run-level' label to be empty, but got %s", runLevel)

By("Check scc of cvo pod.")
podList, err := kubeClient.CoreV1().Pods(cvoNamespace).List(ctx, metav1.ListOptions{
LabelSelector: "k8s-app=cluster-version-operator",
FieldSelector: "status.phase=Running",
})
Expect(err).NotTo(HaveOccurred(), "Failed to list running CVO pods")
Expect(podList.Items).To(HaveLen(1), "Expected exactly one running CVO pod, but found: %d", len(podList.Items))

cvoPod := podList.Items[0]
sccAnnotation := cvoPod.ObjectMeta.Annotations["openshift.io/scc"]
Expect(sccAnnotation).To(Equal("hostaccess"), "Expected SCC for pod %s to be 'hostaccess', but got %s", cvoPod.Name, sccAnnotation)
})
})
})