From 1665c96eb02e709a68cd2cfd8ae634a679081e09 Mon Sep 17 00:00:00 2001 From: Jan Breitkopf Date: Mon, 26 Jan 2026 18:24:19 +0100 Subject: [PATCH] operator: fix OSD info This fixes OSD info being incomplete when a sidecar container is first in the .spec.Template.Spec.Containers list. I added a small helper function to find a container named "osd" instead of always assuming the first one. Signed-off-by: Jan Breitkopf (cherry picked from commit 1ff6f703116b3774d3bc824989877e86036be5ff) --- pkg/operator/ceph/cluster/osd/osd.go | 16 +++++- pkg/operator/ceph/cluster/osd/osd_test.go | 5 +- pkg/operator/ceph/cluster/osd/spec.go | 20 ++++++-- pkg/operator/ceph/cluster/osd/spec_test.go | 52 +++++++++++++------- pkg/operator/ceph/cluster/osd/update_test.go | 6 ++- 5 files changed, 73 insertions(+), 26 deletions(-) diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index c155dc600..619252e8a 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/pkg/operator/ceph/cluster/osd/osd.go @@ -694,8 +694,20 @@ func GetOSDID(d *appsv1.Deployment) (int, error) { return osdID, nil } +func findOSDContainer(containers []corev1.Container) (*corev1.Container, error) { + for i := range containers { + if containers[i].Name == "osd" { + return &containers[i], nil + } + } + return nil, errors.New("OSD container not found in deployment") +} + func (c *Cluster) getOSDInfo(d *appsv1.Deployment) (OSDInfo, error) { - container := d.Spec.Template.Spec.Containers[0] + container, err := findOSDContainer(d.Spec.Template.Spec.Containers) + if err != nil { + return OSDInfo{}, errors.Wrapf(err, "failed to find OSD container in deployment %q", d.Name) + } var osd OSDInfo osdID, err := GetOSDID(d) @@ -706,7 +718,7 @@ func (c *Cluster) getOSDInfo(d *appsv1.Deployment) (OSDInfo, error) { isPVC := false - for _, envVar := range d.Spec.Template.Spec.Containers[0].Env { + for _, envVar := range container.Env { if envVar.Name == "ROOK_NODE_NAME" { osd.NodeName = envVar.Value } diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index b150f97d7..1b878bc3e 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/pkg/operator/ceph/cluster/osd/osd_test.go @@ -763,8 +763,9 @@ func TestGetOSDInfo(t *testing.T) { // set the deployment to have bad info d3, err := c.makeDeployment(osdProp, osd3, dataPathMap) assert.NoError(t, err) - d3.Spec.Template.Spec.Containers[0].Env = append(d3.Spec.Template.Spec.Containers[0].Env, - corev1.EnvVar{Name: blockPathVarName, Value: ""}) + container, err := findOSDContainer(d3.Spec.Template.Spec.Containers) + assert.NoError(t, err) + container.Env = append(container.Env, corev1.EnvVar{Name: blockPathVarName, Value: ""}) _, err = c.getOSDInfo(d3) assert.Error(t, err) }) diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index 64e86cdeb..c499771aa 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -717,7 +717,11 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd *OSDInfo, provision // add OSD container port only if service export is enabled if osd.ExportService { - podTemplateSpec.Spec.Containers[0].Ports = c.getOSDContainerPorts() + container, err := findOSDContainer(podTemplateSpec.Spec.Containers) + if err != nil { + return nil, errors.Wrap(err, "failed to find OSD container") + } + container.Ports = c.getOSDContainerPorts() } // If the log collector is enabled we add the side-car container @@ -730,8 +734,12 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd *OSDInfo, provision podTemplateSpec.Spec.Containers = append(podTemplateSpec.Spec.Containers, *controller.LogCollectorContainer(fmt.Sprintf("ceph-osd.%s", osdID), c.clusterInfo.Namespace, c.spec, nil)) } - podTemplateSpec.Spec.Containers[0] = opconfig.ConfigureStartupProbe(podTemplateSpec.Spec.Containers[0], c.spec.HealthCheck.StartupProbe[cephv1.KeyOSD]) - podTemplateSpec.Spec.Containers[0] = opconfig.ConfigureLivenessProbe(podTemplateSpec.Spec.Containers[0], c.spec.HealthCheck.LivenessProbe[cephv1.KeyOSD]) + container, err := findOSDContainer(podTemplateSpec.Spec.Containers) + if err != nil { + return nil, errors.Wrap(err, "failed to find OSD container") + } + *container = opconfig.ConfigureStartupProbe(*container, c.spec.HealthCheck.StartupProbe[cephv1.KeyOSD]) + *container = opconfig.ConfigureLivenessProbe(*container, c.spec.HealthCheck.LivenessProbe[cephv1.KeyOSD]) if c.spec.Network.IsHost() { podTemplateSpec.Spec.DNSPolicy = v1.DNSClusterFirstWithHostNet @@ -815,7 +823,11 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd *OSDInfo, provision osdAnnotations := cephv1.GetOSDAnnotations(c.spec.Annotations) tcmallocMaxTotalThreadCacheBytes, ok := osdAnnotations[tcmallocMaxTotalThreadCacheBytesEnv] if ok && tcmallocMaxTotalThreadCacheBytes != "" { - deployment.Spec.Template.Spec.Containers[0].Env = append(deployment.Spec.Template.Spec.Containers[0].Env, getTcmallocMaxTotalThreadCacheBytes(tcmallocMaxTotalThreadCacheBytes)) + container, err := findOSDContainer(deployment.Spec.Template.Spec.Containers) + if err != nil { + return nil, errors.Wrap(err, "failed to find OSD container") + } + container.Env = append(container.Env, getTcmallocMaxTotalThreadCacheBytes(tcmallocMaxTotalThreadCacheBytes)) } return deployment, nil diff --git a/pkg/operator/ceph/cluster/osd/spec_test.go b/pkg/operator/ceph/cluster/osd/spec_test.go index 8acb5afdb..aa5f70910 100644 --- a/pkg/operator/ceph/cluster/osd/spec_test.go +++ b/pkg/operator/ceph/cluster/osd/spec_test.go @@ -187,7 +187,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, 8, len(initCont.VolumeMounts)) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont := deployment.Spec.Template.Spec.Containers[0] + cont, err := findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, spec.CephVersion.Image, cont.Image) assert.Equal(t, 8, len(cont.VolumeMounts)) assert.Equal(t, "ceph-osd", cont.Command[0]) @@ -221,7 +222,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, 5, len(initCont.VolumeMounts), initCont.VolumeMounts) blkInitCont := deployment.Spec.Template.Spec.InitContainers[2] assert.Equal(t, 1, len(blkInitCont.VolumeDevices)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 9, len(cont.VolumeMounts), cont.VolumeMounts) // Test OSD on PVC with RAW @@ -239,7 +241,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[3].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[4].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 7, len(cont.VolumeMounts), cont.VolumeMounts) // Test with encrypted OSD on PVC with RAW @@ -260,7 +263,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[7].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[8].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 8, len(cont.VolumeMounts), cont.VolumeMounts) osdProp.encrypted = false assert.Equal(t, 11, len(deployment.Spec.Template.Spec.Volumes), deployment.Spec.Template.Spec.Volumes) @@ -282,7 +286,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[4].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[5].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 7, len(cont.VolumeMounts), cont.VolumeMounts) blkInitCont = deployment.Spec.Template.Spec.InitContainers[1] assert.Equal(t, 1, len(blkInitCont.VolumeDevices)) @@ -314,7 +319,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[10].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[11].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 8, len(cont.VolumeMounts), cont.VolumeMounts) blkInitCont = deployment.Spec.Template.Spec.InitContainers[1] assert.Equal(t, 1, len(blkInitCont.VolumeDevices)) @@ -342,7 +348,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[5].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[6].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 7, len(cont.VolumeMounts), cont.VolumeMounts) blkInitCont = deployment.Spec.Template.Spec.InitContainers[1] assert.Equal(t, 1, len(blkInitCont.VolumeDevices)) @@ -378,7 +385,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[13].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[14].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 8, len(cont.VolumeMounts), cont.VolumeMounts) blkInitCont = deployment.Spec.Template.Spec.InitContainers[1] assert.Equal(t, 1, len(blkInitCont.VolumeDevices)) @@ -407,7 +415,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[8].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[9].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 8, len(cont.VolumeMounts), cont.VolumeMounts) assert.Equal(t, 11, len(deployment.Spec.Template.Spec.Volumes), deployment.Spec.Template.Spec.Volumes) // One more than the encryption with k8s for the kek get init container @@ -432,7 +441,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, "cephx-keyring-update", deployment.Spec.Template.Spec.InitContainers[8].Name) assert.Equal(t, "chown-container-data-dir", deployment.Spec.Template.Spec.InitContainers[9].Name) assert.Equal(t, 1, len(deployment.Spec.Template.Spec.Containers)) - cont = deployment.Spec.Template.Spec.Containers[0] + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) assert.Equal(t, 8, len(cont.VolumeMounts), cont.VolumeMounts) assert.Equal(t, 12, len(deployment.Spec.Template.Spec.Volumes), deployment.Spec.Template.Spec.Volumes) @@ -453,16 +463,20 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { osdProp.tuneFastDeviceClass = true deployment, err = c.makeDeployment(osdProp, osd, dataPathMap) assert.NoError(t, err) + cont, _ = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) for _, flag := range defaultTuneFastSettings { - assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Args, flag) + assert.Contains(t, cont.Args, flag) } // Test tune Slow settings when OSD on PVC osdProp.tuneSlowDeviceClass = true deployment, err = c.makeDeployment(osdProp, osd, dataPathMap) assert.NoError(t, err) + cont, err = findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) for _, flag := range defaultTuneSlowSettings { - assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Args, flag) + assert.Contains(t, cont.Args, flag) } // Test shareProcessNamespace presence @@ -509,10 +523,12 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { deployment, err := c.makeDeployment(osdProp, osd, dataPathMap) assert.Nil(t, err) assert.NotNil(t, deployment) - assert.NotNil(t, deployment.Spec.Template.Spec.Containers[0].LivenessProbe) - assert.NotNil(t, deployment.Spec.Template.Spec.Containers[0].StartupProbe) - assert.Equal(t, int32(900), deployment.Spec.Template.Spec.Containers[0].LivenessProbe.InitialDelaySeconds) - assert.Equal(t, int32(1000), deployment.Spec.Template.Spec.Containers[0].StartupProbe.InitialDelaySeconds) + cont, err := findOSDContainer(deployment.Spec.Template.Spec.Containers) + assert.NoError(t, err) + assert.NotNil(t, cont.LivenessProbe) + assert.NotNil(t, cont.StartupProbe) + assert.Equal(t, int32(900), cont.LivenessProbe.InitialDelaySeconds) + assert.Equal(t, int32(1000), cont.StartupProbe.InitialDelaySeconds) }) // test custom topology label @@ -672,7 +688,9 @@ func TestHostNetwork(t *testing.T) { assert.Equal(t, "rook-ceph-osd-0", r.ObjectMeta.Name) assert.Equal(t, true, r.Spec.Template.Spec.HostNetwork) assert.Equal(t, corev1.DNSClusterFirstWithHostNet, r.Spec.Template.Spec.DNSPolicy) - assert.Equal(t, "2345", r.Spec.Template.Spec.Containers[0].Resources.Limits.Memory().String()) + cont, err := findOSDContainer(r.Spec.Template.Spec.Containers) + assert.NoError(t, err) + assert.Equal(t, "2345", cont.Resources.Limits.Memory().String()) } func TestOsdPrepareResources(t *testing.T) { diff --git a/pkg/operator/ceph/cluster/osd/update_test.go b/pkg/operator/ceph/cluster/osd/update_test.go index c3eb05d7c..8288555af 100644 --- a/pkg/operator/ceph/cluster/osd/update_test.go +++ b/pkg/operator/ceph/cluster/osd/update_test.go @@ -477,7 +477,11 @@ func Test_updateExistingOSDs(t *testing.T) { if err != nil { panic(err) } - d.Spec.Template.Spec.Containers[0].Env = []corev1.EnvVar{} + container, err := findOSDContainer(d.Spec.Template.Spec.Containers) + if err != nil { + panic(err) + } + container.Env = []corev1.EnvVar{} _, err = deploymentClient.Update(context.TODO(), d, metav1.UpdateOptions{}) if err != nil { panic(err)