Skip to content
Open
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
104 changes: 104 additions & 0 deletions test/extended/node/node_e2e/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilrand "k8s.io/apimachinery/pkg/util/rand"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/util/retry"
e2e "k8s.io/kubernetes/test/e2e/framework"

nodeutils "github.com/openshift/origin/test/extended/node"
exutil "github.com/openshift/origin/test/extended/util"
operator "github.com/openshift/origin/test/extended/util/operator"
)

var _ = g.Describe("[sig-node] [Jira:Node/Kubelet] Kubelet, CRI-O, CPU manager", func() {
Expand Down Expand Up @@ -310,3 +312,105 @@ var _ = g.Describe("[sig-node][Suite:openshift/disruptive-longrunning][Disruptiv
"registry.redhat.io/rhel8 should be blocked (NeverContactSource)")
})
})

var _ = g.Describe("[Suite:openshift/disruptive-longrunning][sig-node][Disruptive] Image registry config", func() {
var (
oc = exutil.NewCLIWithoutNamespace("imgcfg")
)

g.BeforeEach(func() {
isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient())
if err != nil {
e2e.Logf("Failed to detect MicroShift cluster: %v", err)
g.Skip("Skipping: unable to determine cluster type")
}
if isMicroShift {
g.Skip("Skipping test on MicroShift cluster - MachineConfig resources are not available")
}
})

// Verifies that updating image.config.openshift.io/cluster with a new search
// registry triggers an MCO rollout and the change lands on nodes.
//author: cmaurya@redhat.com
g.It("[OTP] change container registry config [OCP-44820]", func() {
ctx := context.Background()
searchRegistry := "qe.quay.io"

g.By("Save the original image.config for later restore")
originalImageConfig, err := oc.AdminConfigClient().ConfigV1().Images().Get(ctx, "cluster", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), "failed to get image.config.openshift.io/cluster")

initialWorkerSpec := imagepolicy.GetMCPCurrentSpecConfigName(oc, "worker")
initialMasterSpec := imagepolicy.GetMCPCurrentSpecConfigName(oc, "master")

g.DeferCleanup(func() {
e2e.Logf("Cleanup: restoring original image.config")
restoreErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
current, getErr := oc.AdminConfigClient().ConfigV1().Images().Get(ctx, "cluster", metav1.GetOptions{})
if getErr != nil {
return getErr
}
current.Spec.RegistrySources = originalImageConfig.Spec.RegistrySources
_, updateErr := oc.AdminConfigClient().ConfigV1().Images().Update(ctx, current, metav1.UpdateOptions{})
return updateErr
})
if restoreErr != nil {
e2e.Logf("WARNING: failed to restore original image.config: %v; skipping MCP convergence wait", restoreErr)
return
}

cleanupWorkerSpec := imagepolicy.GetMCPCurrentSpecConfigName(oc, "worker")
cleanupMasterSpec := imagepolicy.GetMCPCurrentSpecConfigName(oc, "master")
imagepolicy.WaitForMCPConfigSpecChangeAndUpdated(oc, "worker", cleanupWorkerSpec)
imagepolicy.WaitForMCPConfigSpecChangeAndUpdated(oc, "master", cleanupMasterSpec)

e2e.Logf("Cleanup: waiting for all cluster operators to settle")
if waitErr := operator.WaitForOperatorsToSettle(ctx, oc.AdminConfigClient(), 10); waitErr != nil {
e2e.Logf("WARNING: cluster operators did not settle after restore: %v", waitErr)
}
})

g.By("Update image.config to add search registry and allowed registries")
err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
imageConfig, getErr := oc.AdminConfigClient().ConfigV1().Images().Get(ctx, "cluster", metav1.GetOptions{})
if getErr != nil {
return getErr
}
imageConfig.Spec.RegistrySources.AllowedRegistries = []string{
"registry.access.redhat.com", "docker.io", "quay.io", searchRegistry,
"image-registry.openshift-image-registry.svc:5000",
}
imageConfig.Spec.RegistrySources.ContainerRuntimeSearchRegistries = []string{
"registry.access.redhat.com", "docker.io", "quay.io", searchRegistry,
}
_, updateErr := oc.AdminConfigClient().ConfigV1().Images().Update(ctx, imageConfig, metav1.UpdateOptions{})
return updateErr
})
o.Expect(err).NotTo(o.HaveOccurred(), "failed to update image.config.openshift.io/cluster")
Comment on lines +373 to +389
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against nil RegistrySources to prevent panic.

Lines 379 and 383 set fields on imageConfig.Spec.RegistrySources without checking if it's nil. In the OpenShift configv1 API, ImageSpec.RegistrySources is a pointer and may be nil on a fresh cluster or after certain operations. Accessing fields on a nil pointer will cause a runtime panic.

🛡️ Proposed fix to initialize RegistrySources if nil
 		g.By("Update image.config to add search registry and allowed registries")
 		err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 			imageConfig, getErr := oc.AdminConfigClient().ConfigV1().Images().Get(ctx, "cluster", metav1.GetOptions{})
 			if getErr != nil {
 				return getErr
 			}
+			if imageConfig.Spec.RegistrySources == nil {
+				imageConfig.Spec.RegistrySources = &configv1.RegistrySources{}
+			}
 			imageConfig.Spec.RegistrySources.AllowedRegistries = []string{
 				"registry.access.redhat.com", "docker.io", "quay.io", searchRegistry,
 				"image-registry.openshift-image-registry.svc:5000",
 			}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/node/node_e2e/node.go` around lines 373 - 389, The code assigns
to imageConfig.Spec.RegistrySources fields without nil-checking; inside the
retry.RetryOnConflict block (where you call
oc.AdminConfigClient().ConfigV1().Images().Get and later Update), check if
imageConfig.Spec.RegistrySources is nil and if so initialize it (e.g., set
imageConfig.Spec.RegistrySources = &configv1.RegistrySources{} ) before setting
AllowedRegistries and ContainerRuntimeSearchRegistries to avoid a runtime panic
when RegistrySources is nil.


g.By("Wait for worker and master MCP rollout to complete")
imagepolicy.WaitForMCPConfigSpecChangeAndUpdated(oc, "worker", initialWorkerSpec)
imagepolicy.WaitForMCPConfigSpecChangeAndUpdated(oc, "master", initialMasterSpec)

g.By("Verify search registries config on a worker node")
workers, err := exutil.GetReadySchedulableWorkerNodes(ctx, oc.AdminKubeClient())
o.Expect(err).NotTo(o.HaveOccurred(), "failed to get ready schedulable worker nodes")
o.Expect(workers).NotTo(o.BeEmpty(), "no ready worker nodes found")

var registriesConf string
o.Eventually(func() error {
var execErr error
registriesConf, execErr = nodeutils.ExecOnNodeWithChroot(oc, workers[0].Name,
"cat", "/etc/containers/registries.conf.d/01-image-searchRegistries.conf")
if execErr != nil {
return execErr
}
if !strings.Contains(registriesConf, searchRegistry) {
return fmt.Errorf("search registry %s not yet in config", searchRegistry)
}
return nil
}, 30*time.Second, 5*time.Second).Should(o.Succeed(),
"search registry %s not found in registries config on node %s", searchRegistry, workers[0].Name)
e2e.Logf("Registries config on %s:\n%s", workers[0].Name, registriesConf)
})
})