From d3398916beffa3a603ec7f90aa88eed37da5e7c0 Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Mon, 1 Dec 2025 15:25:07 -0300 Subject: [PATCH 1/4] fix: extract k8s conformance tests for OCP 4.20+ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added init container to dynamically extract Kubernetes conformance tests from the OTE k8s-tests-ext binary to support OCP 4.20+ where the kubernetes/conformance suite was removed from openshift-tests. Changes: - Added extract-k8s-conformance-tests init container - Extracts k8s-tests-ext.gz from hyperkube image - Filters conformance tests using jq or grep as fallback - Saves test list to /tmp/shared/k8s-conformance-tests.list - Tests container uses extracted list via entrypoint-tests.sh - Gracefully falls back to default suite if extraction fails This ensures the kubernetes/conformance suite continues to work on OCP 4.20+ clusters while maintaining backward compatibility with earlier versions. Related: https://github.com/redhat-openshift-ecosystem/provider-certification-plugins/pull/82 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../plugins/openshift-kube-conformance.yaml | 76 ++++++++++++++++++- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/data/templates/plugins/openshift-kube-conformance.yaml b/data/templates/plugins/openshift-kube-conformance.yaml index 82c9b360..1d97d63c 100644 --- a/data/templates/plugins/openshift-kube-conformance.yaml +++ b/data/templates/plugins/openshift-kube-conformance.yaml @@ -17,6 +17,76 @@ podSpec: volumeMounts: - mountPath: /tmp/shared name: shared + - name: extract-k8s-conformance-tests + image: "{{ .OpenshiftTestsImage }}" + imagePullPolicy: Always + command: + - "/bin/bash" + - "-ec" + - | + set -o pipefail + echo "Extracting Kubernetes conformance test list from OTE binary..." + + # Get the hyperkube image from the release + TESTS_IMAGE_K8S=$( + oc adm release info --image-for=hyperkube 2>/dev/null || echo "" + ) + + if [ -z "$TESTS_IMAGE_K8S" ]; then + echo "Warning: Unable to get hyperkube image" + echo " Will rely on default suite" + exit 0 + fi + + echo "Hyperkube image: $TESTS_IMAGE_K8S" + + # Extract k8s-tests-ext binary + K8S_EXT_FILE="/usr/bin/k8s-tests-ext.gz" + K8S_EXT_PATH="$K8S_EXT_FILE:/tmp/shared/" + if ! oc image extract "$TESTS_IMAGE_K8S" \ + --file="$K8S_EXT_FILE" \ + --path="$K8S_EXT_PATH" 2>/dev/null; then + echo "Warning: Failed to extract k8s-tests-ext.gz" + echo " Will rely on default suite" + exit 0 + fi + + # Decompress and make executable + if [ -f /tmp/shared/k8s-tests-ext.gz ]; then + gunzip /tmp/shared/k8s-tests-ext.gz + chmod u+x /tmp/shared/k8s-tests-ext + + # Extract conformance test list and save to file + echo "Extracting conformance tests list..." + OUT_FILE="/tmp/shared/k8s-conformance-tests.list" + JQ_FILTER='.[] | select(.name | contains("[Conformance]")).name' + /tmp/shared/k8s-tests-ext list \ + | jq -r "$JQ_FILTER" > "$OUT_FILE" 2>/dev/null || { + echo "Warning: Failed to extract with jq, trying grep..." + JSON_FILE="/tmp/shared/k8s-conformance-tests.json" + /tmp/shared/k8s-tests-ext list > "$JSON_FILE" + GREP_PATTERN='"name":"[^"]*\[Conformance\][^"]*"' + grep -o "$GREP_PATTERN" "$JSON_FILE" \ + | sed 's/"name":"//;s/"$//' > "$OUT_FILE" || { + echo "Warning: Failed to extract conformance tests" + echo " Will rely on default suite" + exit 0 + } + } + + TEST_COUNT=$(wc -l < /tmp/shared/k8s-conformance-tests.list) + echo "Extracted $TEST_COUNT conformance tests" + + if [ "$TEST_COUNT" -gt 0 ]; then + echo "Kubernetes conformance test extraction successful" + fi + fi + env: + - name: KUBECONFIG + value: "/tmp/shared/kubeconfig" + volumeMounts: + - mountPath: /tmp/shared + name: shared - name: login image: "{{ .OpenshiftTestsImage }}" imagePullPolicy: Always @@ -64,9 +134,9 @@ sonobuoy-config: description: | OPCT plugin to schedule e2e tests using openshift-tests tool to validate an OpenShift Container Platform cluster installed in a specific provider. - source-url: - "https://github.com/redhat-openshift-ecosystem/opct/\ - blob/main/manifests/openshift-kube-conformance.yaml" + source-url: >- + https://github.com/redhat-openshift-ecosystem/opct/ + blob/main/manifests/openshift-kube-conformance.yaml skipCleanup: true spec: name: plugin From 15e3db8687c532b91069c9a56dbdda2588b965fe Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Tue, 2 Dec 2025 15:07:39 -0300 Subject: [PATCH 2/4] enforcing suite name depending of the cluster version --- .../plugins/openshift-kube-conformance.yaml | 68 +++++++++++------ pkg/run/run.go | 73 ++++++++++++++++--- 2 files changed, 109 insertions(+), 32 deletions(-) diff --git a/data/templates/plugins/openshift-kube-conformance.yaml b/data/templates/plugins/openshift-kube-conformance.yaml index 1d97d63c..de174f4d 100644 --- a/data/templates/plugins/openshift-kube-conformance.yaml +++ b/data/templates/plugins/openshift-kube-conformance.yaml @@ -17,6 +17,28 @@ podSpec: volumeMounts: - mountPath: /tmp/shared name: shared + - name: login + image: "{{ .OpenshiftTestsImage }}" + imagePullPolicy: Always + command: + - "/bin/bash" + - "-c" + - | + /usr/bin/oc login "${KUBE_API_URL}" \ + --token="$(cat "${SA_TOKEN_PATH}")" \ + --certificate-authority="${SA_CA_PATH}"; + env: + - name: KUBECONFIG + value: "/tmp/shared/kubeconfig" + - name: KUBE_API_URL + value: "https://kubernetes.default.svc:443" + - name: SA_TOKEN_PATH + value: "/var/run/secrets/kubernetes.io/serviceaccount/token" + - name: SA_CA_PATH + value: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" + volumeMounts: + - mountPath: /tmp/shared + name: shared - name: extract-k8s-conformance-tests image: "{{ .OpenshiftTestsImage }}" imagePullPolicy: Always @@ -25,6 +47,15 @@ podSpec: - "-ec" - | set -o pipefail + + # We want to skip the extraction on OCP early 4.20 releases, + # so that we can rely on the default suite. + DEFAULT_SUITE_NAME="${DEFAULT_SUITE_NAME:-kubernetes/conformance}" + if [ "${DEFAULT_SUITE_NAME}" == "kubernetes/conformance" ]; then + echo "Skipping extraction of Kubernetes conformance test list from OTE binary..." + exit 0 + fi + echo "Extracting Kubernetes conformance test list from OTE binary..." # Get the hyperkube image from the release @@ -84,28 +115,11 @@ podSpec: env: - name: KUBECONFIG value: "/tmp/shared/kubeconfig" - volumeMounts: - - mountPath: /tmp/shared - name: shared - - name: login - image: "{{ .OpenshiftTestsImage }}" - imagePullPolicy: Always - command: - - "/bin/bash" - - "-c" - - | - /usr/bin/oc login "${KUBE_API_URL}" \ - --token="$(cat "${SA_TOKEN_PATH}")" \ - --certificate-authority="${SA_CA_PATH}"; - env: - - name: KUBECONFIG - value: "/tmp/shared/kubeconfig" - - name: KUBE_API_URL - value: "https://kubernetes.default.svc:443" - - name: SA_TOKEN_PATH - value: "/var/run/secrets/kubernetes.io/serviceaccount/token" - - name: SA_CA_PATH - value: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" + - name: DEFAULT_SUITE_NAME + valueFrom: + configMapKeyRef: + name: plugins-config + key: suiteName volumeMounts: - mountPath: /tmp/shared name: shared @@ -121,7 +135,10 @@ podSpec: - name: KUBECONFIG value: /tmp/shared/kubeconfig - name: DEFAULT_SUITE_NAME - value: "kubernetes/conformance" + valueFrom: + configMapKeyRef: + name: plugins-config + key: suiteNameKubernetesConformance - name: OT_RUN_COMMAND value: "run" - name: PLUGIN_NAME @@ -186,3 +203,8 @@ spec: name: plugins-config key: mirror-registry optional: true + - name: DEFAULT_SUITE_NAME + valueFrom: + configMapKeyRef: + name: plugins-config + key: suiteName diff --git a/pkg/run/run.go b/pkg/run/run.go index 96f1a8c7..288689f8 100644 --- a/pkg/run/run.go +++ b/pkg/run/run.go @@ -487,15 +487,18 @@ func (r *RunOptions) Run(kclient kubernetes.Interface, sclient sonobuoyclient.In configMapData["mirror-registry"] = r.imageRepository } - if err := r.createConfigMap(kclient, sclient, &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: pkg.PluginsVarsConfigMapName, - Namespace: pkg.CertificationNamespace, - }, - Data: configMapData, - }); err != nil { - return err - } + if err := r.setSuiteName(configMapData); err != nil { + return err + } + + if err := r.createConfigMap(kclient, sclient, &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: pkg.PluginsVarsConfigMapName, + Namespace: pkg.CertificationNamespace, + }, + Data: configMapData, + }); err != nil { + return err } if r.plugins == nil || len(*r.plugins) == 0 { @@ -669,3 +672,55 @@ func checkRegistry(irClient irclient.Interface) (bool, error) { return true, nil } + +// setSuiteName sets the suiteNameKubernetesConformance in the configMapData based on the cluster version. +func (r *RunOptions) setSuiteName(configMapData map[string]string) error { + // Gather cluster version to check if it is 4.20+ + // If 4.20+, set the suiteNameKubernetesConformance in the configMapData to "kubernetes/conformance/parallel", + // otherwise set it to "kubernetes/conformance". + // https://issues.redhat.com/browse/OCPBUGS-66219 + suiteNameKubernetesConformance := "kubernetes/conformance" + suiteNameKubernetesConformanceParallel := "kubernetes/conformance/parallel" + + // Get the cluster version + restConfig, err := client.CreateRestConfig() + if err != nil { + return fmt.Errorf("error creating rest config: %w", err) + } + oc, err := coclient.NewForConfig(restConfig) + if err != nil { + return fmt.Errorf("error creating config client: %w", err) + } + cv, err := oc.ConfigV1().ClusterVersions().Get(context.TODO(), "version", metav1.GetOptions{}) + if err != nil { + log.Warnf("Failed to get cluster version, defaulting to kubernetes/conformance suite with openshift-tests: %v", err) + return nil + } + + // Extract the version string + version := cv.Status.Desired.Version + if version == "" { + log.Warn("Cluster version is empty, defaulting to kubernetes/conformance suite with openshift-tests") + return nil + } + + log.Debugf("Detected cluster version: %s", version) + + // Parse the version to check if it's >= 4.20 + // Version format is typically "4.20.0" or "4.20.0-rc.1" + var major, minor int + _, err = fmt.Sscanf(version, "%d.%d", &major, &minor) + if err != nil { + log.Warnf("Failed to parse cluster version %q, defaulting to kubernetes/conformance suite with openshift-tests: %v", version, err) + return nil + } + + // For OCP 4.20+, use k8s-tests-ext binary with parallel sub-suite + configMapData["suiteNameKubernetesConformance"] = suiteNameKubernetesConformance + if major == 4 && minor >= 20 { + configMapData["suiteNameKubernetesConformance"] = suiteNameKubernetesConformanceParallel + } + log.Infof("Setting configMapData[suiteNameKubernetesConformance] to %s for OCP %d.%d", configMapData["suiteNameKubernetesConformance"], major, minor) + + return nil +} From 290ff432f2b09dbdc958a8cb7d27e0f4abcd9c7a Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Tue, 2 Dec 2025 18:22:59 -0300 Subject: [PATCH 3/4] Update clients to propagate to the runner --- .containerignore | 64 ++++ .gitignore | 1 + .../plugins/openshift-kube-conformance.yaml | 4 +- pkg/client/client.go | 65 ++-- pkg/client/client_test.go | 307 ++++++++++++++++++ pkg/cmd/adm/e2ed/taintNode.go | 12 +- pkg/destroy/destroy.go | 8 +- pkg/run/run.go | 120 ++++--- pkg/run/validations.go | 10 +- pkg/status/status.go | 9 +- 10 files changed, 498 insertions(+), 102 deletions(-) create mode 100644 .containerignore create mode 100644 pkg/client/client_test.go diff --git a/.containerignore b/.containerignore new file mode 100644 index 00000000..40e89b07 --- /dev/null +++ b/.containerignore @@ -0,0 +1,64 @@ +# Build artifacts +build/ +dist/ + +# IDE and editor files +.idea/ +.vscode/ +*.swp +*.swo +*~ + +# Git files +.git/ +.gitignore +.gitattributes + +# Documentation (not needed in container) +docs/ +*.md +!README.md + +# Test files and test data +test/ +*_test.go +testdata/ + +# Development and CI files +hack/ +.github/ +.gitlab-ci.yml +.travis.yml +.circleci/ + +# Python virtual environments +.venv/ +venv/ +__pycache__/ +*.pyc + +# Log files +*.log +opct.log + +# Generated files +docs/CHANGELOG.md +docs/CHANGELOG_commits.md +docs/CHANGELOG_summary.md + +# Configuration files not needed in container +kubeconfig +*.kubeconfig + +# OS files +.DS_Store +Thumbs.db + +# MkDocs build files +site/ +.mkdocs/ + +# Temporary files +tmp/ +temp/ +*.tmp diff --git a/.gitignore b/.gitignore index ea6c5dca..373ba538 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ build/ # See the make commands: 'build-changelog' and 'build-changelog-commmits' docs/CHANGELOG.md docs/CHANGELOG_commits.md +opct.log diff --git a/data/templates/plugins/openshift-kube-conformance.yaml b/data/templates/plugins/openshift-kube-conformance.yaml index de174f4d..e93de6cb 100644 --- a/data/templates/plugins/openshift-kube-conformance.yaml +++ b/data/templates/plugins/openshift-kube-conformance.yaml @@ -119,7 +119,7 @@ podSpec: valueFrom: configMapKeyRef: name: plugins-config - key: suiteName + key: suiteNameKubernetesConformance volumeMounts: - mountPath: /tmp/shared name: shared @@ -207,4 +207,4 @@ spec: valueFrom: configMapKeyRef: name: plugins-config - key: suiteName + key: suiteNameKubernetesConformance diff --git a/pkg/client/client.go b/pkg/client/client.go index 9701b952..72fe4ae2 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -12,45 +12,58 @@ import ( "k8s.io/client-go/tools/clientcmd" ) -func CreateRestConfig() (*rest.Config, error) { - kubeconfig := os.Getenv("KUBECONFIG") - if len(kubeconfig) == 0 { - kubeconfig = viper.GetString("kubeconfig") - if kubeconfig == "" { - return nil, fmt.Errorf("--kubeconfig or KUBECONFIG environment variable must be set") - } - - // Check kubeconfig exists - if _, err := os.Stat(kubeconfig); err != nil { - return nil, fmt.Errorf("kubeconfig %q does not exists: %v", kubeconfig, err) - } - } - - clientConfig, err := clientcmd.BuildConfigFromFlags("", kubeconfig) - return clientConfig, err +// Client is the interface to store kubernetes and sonobuoy client instances. +type Client struct { + // KClient is the kubernetes client instance. + KClient kubernetes.Interface + // SClient is the sonobuoy client instance. + SClient client.Interface + // RestConfig is the rest config for the kubernetes client. + RestConfig *rest.Config } -// CreateClients creates kubernetes and sonobuoy client instances -func CreateClients() (kubernetes.Interface, client.Interface, error) { - clientConfig, err := CreateRestConfig() +// NewClient creates a new client instance. +func NewClient() (*Client, error) { + clientConfig, err := createRestConfig() if err != nil { - return nil, nil, fmt.Errorf("error creating kube client config: %v", err) + return nil, fmt.Errorf("error creating rest config: %v", err) + } + cli := &Client{ + RestConfig: clientConfig, } - clientset, err := kubernetes.NewForConfig(clientConfig) + cli.KClient, err = kubernetes.NewForConfig(clientConfig) if err != nil { - return nil, nil, fmt.Errorf("error creating kube client: %v", err) + return cli, fmt.Errorf("error creating kube client: %v", err) } skc, err := sonodynamic.NewAPIHelperFromRESTConfig(clientConfig) if err != nil { - return nil, nil, fmt.Errorf("error creating sonobuoy rest helper: %v", err) + return cli, fmt.Errorf("error creating sonobuoy rest helper: %v", err) } - sonobuoyClient, err := client.NewSonobuoyClient(clientConfig, skc) + cli.SClient, err = client.NewSonobuoyClient(clientConfig, skc) if err != nil { - return nil, nil, fmt.Errorf("error creating sonobuoy client: %v", err) + return cli, fmt.Errorf("error creating sonobuoy client: %v", err) } - return clientset, sonobuoyClient, nil + return cli, nil +} + +func createRestConfig() (*rest.Config, error) { + kubeconfig := os.Getenv("KUBECONFIG") + if len(kubeconfig) == 0 { + kubeconfig = viper.GetString("kubeconfig") + if kubeconfig == "" { + return nil, fmt.Errorf("--kubeconfig or KUBECONFIG environment variable must be set") + } + + // Check kubeconfig exists + if _, err := os.Stat(kubeconfig); err != nil { + return nil, fmt.Errorf("kubeconfig %q does not exists: %v", kubeconfig, err) + } + } + + clientConfig, err := clientcmd.BuildConfigFromFlags("", kubeconfig) + return clientConfig, err } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go new file mode 100644 index 00000000..0fe701ee --- /dev/null +++ b/pkg/client/client_test.go @@ -0,0 +1,307 @@ +package client + +import ( + "os" + "path/filepath" + "testing" + + "github.com/spf13/viper" + "k8s.io/client-go/rest" +) + +func TestNewClient(t *testing.T) { + // Note: NewClient() requires a functional Kubernetes cluster to fully succeed + // because it creates the Sonobuoy client which makes API calls. + // These tests focus on error conditions that occur before cluster communication. + + tests := []struct { + name string + setupEnv func() + cleanupEnv func() + wantErr bool + errorContains string + }{ + { + name: "error when no kubeconfig is set", + setupEnv: func() { + os.Unsetenv("KUBECONFIG") + viper.Reset() + }, + cleanupEnv: func() {}, + wantErr: true, + errorContains: "--kubeconfig or KUBECONFIG environment variable must be set", + }, + { + name: "error when kubeconfig file does not exist", + setupEnv: func() { + os.Setenv("KUBECONFIG", "/nonexistent/kubeconfig") + }, + cleanupEnv: func() { + os.Unsetenv("KUBECONFIG") + }, + wantErr: true, + errorContains: "no such file or directory", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.setupEnv() + defer tt.cleanupEnv() + + cli, err := NewClient() + + if tt.wantErr { + if err == nil { + t.Errorf("NewClient() error = nil, wantErr %v", tt.wantErr) + return + } + if tt.errorContains != "" && !contains(err.Error(), tt.errorContains) { + t.Errorf("NewClient() error = %v, want error containing %v", err, tt.errorContains) + } + return + } + + if err != nil { + t.Errorf("NewClient() unexpected error = %v", err) + return + } + + if cli == nil { + t.Error("NewClient() returned nil client") + return + } + + if cli.KClient == nil { + t.Error("NewClient() KClient is nil") + } + + if cli.SClient == nil { + t.Error("NewClient() SClient is nil") + } + + if cli.RestConfig == nil { + t.Error("NewClient() RestConfig is nil") + } + }) + } +} + +func TestCreateRestConfig(t *testing.T) { + // Create a temporary kubeconfig file for testing + tmpDir := t.TempDir() + kubeconfigPath := filepath.Join(tmpDir, "kubeconfig") + + kubeconfigContent := `apiVersion: v1 +kind: Config +clusters: +- cluster: + server: https://localhost:6443 + name: test-cluster +contexts: +- context: + cluster: test-cluster + user: test-user + name: test-context +current-context: test-context +users: +- name: test-user + user: + token: test-token +` + if err := os.WriteFile(kubeconfigPath, []byte(kubeconfigContent), 0644); err != nil { + t.Fatalf("Failed to create test kubeconfig: %v", err) + } + + tests := []struct { + name string + setupEnv func() + cleanupEnv func() + wantErr bool + errorContains string + validateConfig func(*testing.T, *rest.Config) + }{ + { + name: "successful config creation with KUBECONFIG env var", + setupEnv: func() { + os.Setenv("KUBECONFIG", kubeconfigPath) + }, + cleanupEnv: func() { + os.Unsetenv("KUBECONFIG") + }, + wantErr: false, + validateConfig: func(t *testing.T, cfg *rest.Config) { + if cfg.Host != "https://localhost:6443" { + t.Errorf("Expected host https://localhost:6443, got %s", cfg.Host) + } + }, + }, + { + name: "successful config creation with viper kubeconfig", + setupEnv: func() { + os.Unsetenv("KUBECONFIG") + viper.Set("kubeconfig", kubeconfigPath) + }, + cleanupEnv: func() { + viper.Reset() + }, + wantErr: false, + validateConfig: func(t *testing.T, cfg *rest.Config) { + if cfg.Host != "https://localhost:6443" { + t.Errorf("Expected host https://localhost:6443, got %s", cfg.Host) + } + }, + }, + { + name: "error when KUBECONFIG env var is not set and viper is empty", + setupEnv: func() { + os.Unsetenv("KUBECONFIG") + viper.Reset() + }, + cleanupEnv: func() {}, + wantErr: true, + errorContains: "--kubeconfig or KUBECONFIG environment variable must be set", + }, + { + name: "error when kubeconfig file does not exist (KUBECONFIG env)", + setupEnv: func() { + os.Setenv("KUBECONFIG", "/nonexistent/path/kubeconfig") + }, + cleanupEnv: func() { + os.Unsetenv("KUBECONFIG") + }, + wantErr: true, + errorContains: "no such file or directory", + }, + { + name: "error when kubeconfig file does not exist (viper)", + setupEnv: func() { + os.Unsetenv("KUBECONFIG") + viper.Set("kubeconfig", "/another/nonexistent/kubeconfig") + }, + cleanupEnv: func() { + viper.Reset() + }, + wantErr: true, + errorContains: "does not exists", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.setupEnv() + defer tt.cleanupEnv() + + cfg, err := createRestConfig() + + if tt.wantErr { + if err == nil { + t.Errorf("createRestConfig() error = nil, wantErr %v", tt.wantErr) + return + } + if tt.errorContains != "" && !contains(err.Error(), tt.errorContains) { + t.Errorf("createRestConfig() error = %v, want error containing %v", err, tt.errorContains) + } + return + } + + if err != nil { + t.Errorf("createRestConfig() unexpected error = %v", err) + return + } + + if cfg == nil { + t.Error("createRestConfig() returned nil config") + return + } + + if tt.validateConfig != nil { + tt.validateConfig(t, cfg) + } + }) + } +} + +func TestCreateRestConfig_PriorityOrder(t *testing.T) { + // Test that KUBECONFIG env var takes priority over viper config + tmpDir := t.TempDir() + + // Create two different kubeconfig files + envKubeconfigPath := filepath.Join(tmpDir, "env-kubeconfig") + viperKubeconfigPath := filepath.Join(tmpDir, "viper-kubeconfig") + + envContent := `apiVersion: v1 +kind: Config +clusters: +- cluster: + server: https://env-server:6443 + name: env-cluster +contexts: +- context: + cluster: env-cluster + user: env-user + name: env-context +current-context: env-context +users: +- name: env-user + user: + token: env-token +` + viperContent := `apiVersion: v1 +kind: Config +clusters: +- cluster: + server: https://viper-server:6443 + name: viper-cluster +contexts: +- context: + cluster: viper-cluster + user: viper-user + name: viper-context +current-context: viper-context +users: +- name: viper-user + user: + token: viper-token +` + + if err := os.WriteFile(envKubeconfigPath, []byte(envContent), 0644); err != nil { + t.Fatalf("Failed to create env kubeconfig: %v", err) + } + if err := os.WriteFile(viperKubeconfigPath, []byte(viperContent), 0644); err != nil { + t.Fatalf("Failed to create viper kubeconfig: %v", err) + } + + // Set both KUBECONFIG env var and viper config + os.Setenv("KUBECONFIG", envKubeconfigPath) + viper.Set("kubeconfig", viperKubeconfigPath) + defer func() { + os.Unsetenv("KUBECONFIG") + viper.Reset() + }() + + cfg, err := createRestConfig() + if err != nil { + t.Fatalf("createRestConfig() unexpected error = %v", err) + } + + // Should use KUBECONFIG env var (priority) + if cfg.Host != "https://env-server:6443" { + t.Errorf("Expected KUBECONFIG env var to take priority, got host %s", cfg.Host) + } +} + +// Helper function to check if a string contains a substring +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(substr) == 0 || + (len(s) > 0 && len(substr) > 0 && containsHelper(s, substr))) +} + +func containsHelper(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/pkg/cmd/adm/e2ed/taintNode.go b/pkg/cmd/adm/e2ed/taintNode.go index c3606a02..984975ac 100644 --- a/pkg/cmd/adm/e2ed/taintNode.go +++ b/pkg/cmd/adm/e2ed/taintNode.go @@ -116,15 +116,15 @@ func applyTaintToNode(kclient kubernetes.Interface, nodeName string) error { } func taintNodeRun(cmd *cobra.Command, args []string) { - kclient, _, err := client.CreateClients() + cli, err := client.NewClient() if err != nil { - log.Fatalf("Failed to create Kubernetes client: %v", err) + log.WithError(err).Fatalf("Failed to create client") } if taintNodeArgs.nodeName == "" { - taintNodeArgs.nodeName, err = discoverNode(kclient) + taintNodeArgs.nodeName, err = discoverNode(cli.KClient) if err != nil { - log.Fatalf("Failed to discover node: %v", err) + log.WithError(err).Fatalf("Failed to discover node") } } log.Infof("Setting up node %s...", taintNodeArgs.nodeName) @@ -134,7 +134,7 @@ func taintNodeRun(cmd *cobra.Command, args []string) { return } - if err := applyTaintToNode(kclient, taintNodeArgs.nodeName); err != nil { - log.Fatalf("Failed to apply taint to node: %v", err) + if err := applyTaintToNode(cli.KClient, taintNodeArgs.nodeName); err != nil { + log.WithError(err).Fatalf("Failed to apply taint to node") } } diff --git a/pkg/destroy/destroy.go b/pkg/destroy/destroy.go index 3dc1dcde..5afa3753 100644 --- a/pkg/destroy/destroy.go +++ b/pkg/destroy/destroy.go @@ -38,24 +38,24 @@ func NewCmdDestroy() *cobra.Command { log.Info("Starting the destroy flow...") // Client setup - kclient, sclient, err := client.CreateClients() + cli, err := client.NewClient() if err != nil { log.Error(err) return err } log.Info("removing opct namespace...") - if err := o.DeleteSonobuoyEnv(sclient); err != nil { + if err := o.DeleteSonobuoyEnv(cli.SClient); err != nil { log.Warn(err) } log.Info("removing stale e2e namespaces...") - if err := o.DeleteTestNamespaces(kclient); err != nil { + if err := o.DeleteTestNamespaces(cli.KClient); err != nil { log.Warn(err) } log.Info("restoring privileged environment...") - if err := o.RestoreSCC(kclient); err != nil { + if err := o.RestoreSCC(cli.KClient); err != nil { log.Warn(err) } diff --git a/pkg/run/run.go b/pkg/run/run.go index 288689f8..54af2e7f 100644 --- a/pkg/run/run.go +++ b/pkg/run/run.go @@ -98,10 +98,8 @@ func hideOptionalFlags(cmd *cobra.Command, flag string) { } func NewCmdRun() *cobra.Command { - var err error - var kclient kubernetes.Interface - var sclient sonobuoyclient.Interface o := newRunOptions() + var cli *client.Client cmd := &cobra.Command{ Use: "run", @@ -109,19 +107,20 @@ func NewCmdRun() *cobra.Command { Long: `Launches the provider validation environment inside of an already running OpenShift cluster`, PreRunE: func(cmd *cobra.Command, args []string) error { // Client setup - kclient, sclient, err = client.CreateClients() + var err error + cli, err = client.NewClient() if err != nil { log.WithError(err).Error("pre-run failed when creating clients") return err } // Pre-run validations - if errs := o.PreRunValidations(kclient); len(errs) > 0 { + if errs := o.PreRunValidations(cli.KClient, cli.RestConfig); len(errs) > 0 { return fmt.Errorf("pre-run validation failed with %d errors, fix it and try again", len(errs)) } // Pre-run setup - if err = o.PreRunSetup(kclient); err != nil { + if err = o.PreRunSetup(cli.KClient); err != nil { log.WithError(err).Error("pre-run failed when initializing the environment") return err } @@ -129,7 +128,7 @@ func NewCmdRun() *cobra.Command { }, RunE: func(cmd *cobra.Command, args []string) error { log.Info("Running OPCT...") - if err := o.Run(kclient, sclient); err != nil { + if err := o.Run(cli); err != nil { log.WithError(err).Errorf("execution finished with errors.") return err } @@ -140,7 +139,7 @@ func NewCmdRun() *cobra.Command { } log.Info("Jobs scheduled! Waiting for resources be created...") - if err := wait.WaitForRequiredResources(kclient); err != nil { + if err := wait.WaitForRequiredResources(cli.KClient); err != nil { log.WithError(err).Errorf("error waiting for required pods to become ready") return err } @@ -148,8 +147,8 @@ func NewCmdRun() *cobra.Command { // Retrieve the first status and print it, finishing when --watch is not set. s := status.NewStatus(&status.StatusInput{ Watch: o.watch, - KClient: kclient, - SClient: sclient, + KClient: cli.KClient, + SClient: cli.SClient, }) if err := s.WaitForStatusReport(cmd.Context()); err != nil { log.WithError(err).Error("error retrieving aggregator status") @@ -199,6 +198,11 @@ func NewCmdRun() *cobra.Command { cmd.Flags().BoolVar(&o.dedicated, "dedicated", defaultDedicatedFlag, "Setup plugins to run in dedicated test environment.") cmd.Flags().BoolVar(&o.dryRun, "dry-run", defaultDryRunFlag, "Run preflight checks only without creating resources") cmd.Flags().BoolVarP(&o.verbose, "verbose", "v", defaultVerboseFlag, "Print rendered plugin manifests to stdout") + cmd.Flags().BoolVar(&o.devSkipChecks, "devel-skip-checks", false, "Developer Mode only: skip checks") + + // dev-count is an alias of devel-limit-tests (both flags bind to the same variable). + // TODO(mtulio): remove this flag in the future. + cmd.Flags().StringVar(&o.devCount, "devel-limit-tests", "0", "Developer Mode only: run small random set of tests. Default: 0 (disabled)") cmd.Flags().StringVar(&o.devCount, "dev-count", "0", "Developer Mode only: run small random set of tests. Default: 0 (disabled)") hideOptionalFlags(cmd, "plugin") @@ -407,7 +411,7 @@ func (r *RunOptions) PreRunSetup(kclient kubernetes.Interface) error { } // createConfigMap generic way to create the configMap on the certification namespace. -func (r *RunOptions) createConfigMap(kclient kubernetes.Interface, sclient sonobuoyclient.Interface, cm *v1.ConfigMap) error { +func (r *RunOptions) createConfigMap(kclient kubernetes.Interface, cm *v1.ConfigMap) error { _, err := kclient.CoreV1().ConfigMaps(pkg.CertificationNamespace).Create(context.TODO(), cm, metav1.CreateOptions{}) if err != nil { return err @@ -416,7 +420,7 @@ func (r *RunOptions) createConfigMap(kclient kubernetes.Interface, sclient sonob } // Run setup and provision the certification environment. -func (r *RunOptions) Run(kclient kubernetes.Interface, sclient sonobuoyclient.Interface) error { +func (r *RunOptions) Run(cli *client.Client) error { var manifests []*manifest.Manifest imageRepository := pkg.DefaultToolsRepository @@ -444,7 +448,7 @@ func (r *RunOptions) Run(kclient kubernetes.Interface, sclient sonobuoyclient.In } // Let Sonobuoy do some preflight checks before we run - errs := sclient.PreflightChecks(&sonobuoyclient.PreflightConfig{ + errs := cli.SClient.PreflightChecks(&sonobuoyclient.PreflightConfig{ Namespace: pkg.CertificationNamespace, DNSNamespace: "openshift-dns", DNSPodLabels: []string{"dns.operator.openshift.io/daemonset-dns=default"}, @@ -460,47 +464,65 @@ func (r *RunOptions) Run(kclient kubernetes.Interface, sclient sonobuoyclient.In log.Warn("DEVEL MODE, THIS IS NOT SUPPORTED: Skipping preflight checks") } - // Create version information ConfigMap - if !r.dryRun { - if err := r.createConfigMap(kclient, sclient, &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: pkg.VersionInfoConfigMapName, - Namespace: pkg.CertificationNamespace, - }, - Data: map[string]string{ - "cli-version": version.Version.Version, - "cli-commit": version.Version.Commit, - "sonobuoy-version": buildinfo.Version, - "sonobuoy-image": r.sonobuoyImage, - }, - }); err != nil { - return err - } - - configMapData := map[string]string{ + // Create environment configuration ConfigMaps + configVersion := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: pkg.VersionInfoConfigMapName, + Namespace: pkg.CertificationNamespace, + }, + Data: map[string]string{ + "cli-version": version.Version.Version, + "cli-commit": version.Version.Commit, + "sonobuoy-version": buildinfo.Version, + "sonobuoy-image": r.sonobuoyImage, + }, + } + configPlugins := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: pkg.PluginsVarsConfigMapName, + Namespace: pkg.CertificationNamespace, + }, + Data: map[string]string{ "dev-count": r.devCount, "run-mode": r.mode, "upgrade-target-images": r.upgradeImage, - } + }, + } + if len(r.imageRepository) > 0 { + configPlugins.Data["mirror-registry"] = r.imageRepository + } + if err := r.setSuiteName(cli, configPlugins.Data); err != nil { + return err + } - if len(r.imageRepository) > 0 { - configMapData["mirror-registry"] = r.imageRepository + if r.verbose { + versionJSON, err := json.MarshalIndent(configVersion, "", " ") + if err != nil { + log.Warnf("Unable to marshal ConfigMap %s for printing: %v", configVersion.Name, err) + } else { + fmt.Printf("\n---\n# ConfigMap: %s\n---\n%s\n", configVersion.Name, string(versionJSON)) } - if err := r.setSuiteName(configMapData); err != nil { - return err + pluginsJSON, err := json.MarshalIndent(configPlugins, "", " ") + if err != nil { + log.Warnf("Unable to marshal ConfigMap %s for printing: %v", configPlugins.Name, err) + } else { + fmt.Printf("\n---\n# ConfigMap: %s\n---\n%s\n", configPlugins.Name, string(pluginsJSON)) + } } - if err := r.createConfigMap(kclient, sclient, &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: pkg.PluginsVarsConfigMapName, - Namespace: pkg.CertificationNamespace, - }, - Data: configMapData, - }); err != nil { - return err + if r.dryRun { + log.Debugf("Dry-run mode enabled, skipping creation of environment configuration ConfigMaps") + } else { + if err := r.createConfigMap(cli.KClient, configVersion); err != nil { + return err + } + if err := r.createConfigMap(cli.KClient, configPlugins); err != nil { + return err + } } + // Loading Plugins configurations / manifests if r.plugins == nil || len(*r.plugins) == 0 { log.Debugf("Loading default plugins") var err error @@ -568,7 +590,7 @@ func (r *RunOptions) Run(kclient kubernetes.Interface, sclient sonobuoyclient.In return nil } - err := sclient.Run(runConfig) + err := cli.SClient.Run(runConfig) return err } @@ -674,7 +696,7 @@ func checkRegistry(irClient irclient.Interface) (bool, error) { } // setSuiteName sets the suiteNameKubernetesConformance in the configMapData based on the cluster version. -func (r *RunOptions) setSuiteName(configMapData map[string]string) error { +func (r *RunOptions) setSuiteName(cli *client.Client, configMapData map[string]string) error { // Gather cluster version to check if it is 4.20+ // If 4.20+, set the suiteNameKubernetesConformance in the configMapData to "kubernetes/conformance/parallel", // otherwise set it to "kubernetes/conformance". @@ -683,11 +705,7 @@ func (r *RunOptions) setSuiteName(configMapData map[string]string) error { suiteNameKubernetesConformanceParallel := "kubernetes/conformance/parallel" // Get the cluster version - restConfig, err := client.CreateRestConfig() - if err != nil { - return fmt.Errorf("error creating rest config: %w", err) - } - oc, err := coclient.NewForConfig(restConfig) + oc, err := coclient.NewForConfig(cli.RestConfig) if err != nil { return fmt.Errorf("error creating config client: %w", err) } diff --git a/pkg/run/validations.go b/pkg/run/validations.go index e58b6e67..dffeac0e 100644 --- a/pkg/run/validations.go +++ b/pkg/run/validations.go @@ -12,7 +12,6 @@ import ( irclient "github.com/openshift/client-go/imageregistry/clientset/versioned" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" "github.com/redhat-openshift-ecosystem/opct/pkg" - "github.com/redhat-openshift-ecosystem/opct/pkg/client" log "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -22,17 +21,10 @@ import ( ) // PreRunValidations performs some validations before running the environment -func (r *RunOptions) PreRunValidations(kclient kubernetes.Interface) []error { +func (r *RunOptions) PreRunValidations(kclient kubernetes.Interface, restConfig *rest.Config) []error { log.Info("Starting preflight validations") allErrors := []error{} - // Get ConfigV1 client for Cluster Operators - restConfig, err := client.CreateRestConfig() - if err != nil { - log.Errorf("error creating rest config: %v", err) - return []error{err} - } - // Validate if all configured container images are accessible errImages := validateContainerImagesAccessibility([]string{ r.PluginsImage, diff --git a/pkg/status/status.go b/pkg/status/status.go index 974c3902..43ddd17c 100644 --- a/pkg/status/status.go +++ b/pkg/status/status.go @@ -56,19 +56,20 @@ func NewStatus(in *StatusInput) *Status { if in.IntervalSeconds != 0 { s.waitInterval = time.Duration(in.IntervalSeconds) * time.Second } - kclient, sclient, err := client.CreateClients() + cli, err := client.NewClient() if err != nil { - log.WithError(err).Errorf("error creating clients: %v", err) + log.WithError(err).Errorf("error creating client: %v", err) return s } s.kclient = in.KClient if s.kclient == nil { - s.kclient = kclient + s.kclient = cli.KClient } s.sclient = in.SClient if s.sclient == nil { - s.sclient = sclient + s.sclient = cli.SClient } + return s } From 9259d893ac3da156513ffe76e1dcbc930aa5d76d Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Tue, 2 Dec 2025 18:46:26 -0300 Subject: [PATCH 4/4] fix: resolve linter errors in PR #183 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed linter issues found in CI checks: **YAML linting (data/templates/plugins/openshift-kube-conformance.yaml)**: - Fixed line-length error on line 55 (93 > 80 characters) - Split echo message across two lines for better readability **Go linting (pkg/client/client_test.go)**: - Fixed errcheck warnings by handling return values from os.Setenv/os.Unsetenv - Used blank identifier (_) to explicitly ignore errors in test setup/cleanup - These are intentional in test contexts where environment setup failures would be caught by subsequent test assertions Validation: - ✅ yamllint passes for openshift-kube-conformance.yaml - ✅ make test - all tests passing - ✅ make vet - no issues found - ✅ go test ./pkg/client - all client tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../plugins/openshift-kube-conformance.yaml | 3 ++- pkg/client/client_test.go | 24 +++++++++---------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/data/templates/plugins/openshift-kube-conformance.yaml b/data/templates/plugins/openshift-kube-conformance.yaml index e93de6cb..eb12d926 100644 --- a/data/templates/plugins/openshift-kube-conformance.yaml +++ b/data/templates/plugins/openshift-kube-conformance.yaml @@ -52,7 +52,8 @@ podSpec: # so that we can rely on the default suite. DEFAULT_SUITE_NAME="${DEFAULT_SUITE_NAME:-kubernetes/conformance}" if [ "${DEFAULT_SUITE_NAME}" == "kubernetes/conformance" ]; then - echo "Skipping extraction of Kubernetes conformance test list from OTE binary..." + echo "Skipping extraction of Kubernetes conformance test list" \ + "from OTE binary..." exit 0 fi diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 0fe701ee..25f4bda6 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -24,7 +24,7 @@ func TestNewClient(t *testing.T) { { name: "error when no kubeconfig is set", setupEnv: func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") viper.Reset() }, cleanupEnv: func() {}, @@ -34,10 +34,10 @@ func TestNewClient(t *testing.T) { { name: "error when kubeconfig file does not exist", setupEnv: func() { - os.Setenv("KUBECONFIG", "/nonexistent/kubeconfig") + _ = os.Setenv("KUBECONFIG", "/nonexistent/kubeconfig") }, cleanupEnv: func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") }, wantErr: true, errorContains: "no such file or directory", @@ -124,10 +124,10 @@ users: { name: "successful config creation with KUBECONFIG env var", setupEnv: func() { - os.Setenv("KUBECONFIG", kubeconfigPath) + _ = os.Setenv("KUBECONFIG", kubeconfigPath) }, cleanupEnv: func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") }, wantErr: false, validateConfig: func(t *testing.T, cfg *rest.Config) { @@ -139,7 +139,7 @@ users: { name: "successful config creation with viper kubeconfig", setupEnv: func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") viper.Set("kubeconfig", kubeconfigPath) }, cleanupEnv: func() { @@ -155,7 +155,7 @@ users: { name: "error when KUBECONFIG env var is not set and viper is empty", setupEnv: func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") viper.Reset() }, cleanupEnv: func() {}, @@ -165,10 +165,10 @@ users: { name: "error when kubeconfig file does not exist (KUBECONFIG env)", setupEnv: func() { - os.Setenv("KUBECONFIG", "/nonexistent/path/kubeconfig") + _ = os.Setenv("KUBECONFIG", "/nonexistent/path/kubeconfig") }, cleanupEnv: func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") }, wantErr: true, errorContains: "no such file or directory", @@ -176,7 +176,7 @@ users: { name: "error when kubeconfig file does not exist (viper)", setupEnv: func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") viper.Set("kubeconfig", "/another/nonexistent/kubeconfig") }, cleanupEnv: func() { @@ -273,10 +273,10 @@ users: } // Set both KUBECONFIG env var and viper config - os.Setenv("KUBECONFIG", envKubeconfigPath) + _ = os.Setenv("KUBECONFIG", envKubeconfigPath) viper.Set("kubeconfig", viperKubeconfigPath) defer func() { - os.Unsetenv("KUBECONFIG") + _ = os.Unsetenv("KUBECONFIG") viper.Reset() }()