Skip to content

Commit

Permalink
Merge pull request #809 from red-hat-storage/sync_us--master
Browse files Browse the repository at this point in the history
Syncing latest changes from upstream master for rook
  • Loading branch information
subhamkrai authored Jan 29, 2025
2 parents 354d732 + 7031e92 commit 3112a2b
Show file tree
Hide file tree
Showing 24 changed files with 155 additions and 54 deletions.
1 change: 1 addition & 0 deletions Documentation/Helm-Charts/operator-chart.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ The following table lists the configurable parameters of the rook-operator chart
| `csi.topology.domainLabels` | domainLabels define which node labels to use as domains for CSI nodeplugins to advertise their domains | `nil` |
| `csi.topology.enabled` | Enable topology based provisioning | `false` |
| `currentNamespaceOnly` | Whether the operator should watch cluster CRD in its own namespace or not | `false` |
| `customHostnameLabel` | Custom label to identify node hostname. If not set `kubernetes.io/hostname` will be used | `nil` |
| `disableDeviceHotplug` | Disable automatic orchestration when new devices are discovered. | `false` |
| `discover.nodeAffinity` | The node labels for affinity of `discover-agent` [^1] | `nil` |
| `discover.podLabels` | Labels to add to the discover pods | `nil` |
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,12 @@ fmt: ## Check formatting of go sources.
yamllint:
yamllint -c .yamllint deploy/examples/ --no-warnings

.PHONY: lint
lint: yamllint pylint shellcheck vet ## Run various linters

.PHONY: pylint
pylint:
pylint $(shell find $(ROOT_DIR) -name '*.py') -E
pylint $(shell find $(ROOT_DIR) -name '*.py') -E

.PHONY: shellcheck
shellcheck:
Expand Down
4 changes: 4 additions & 0 deletions deploy/charts/rook-ceph/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ spec:
{{- else }}
- name: ROOK_HOSTPATH_REQUIRES_PRIVILEGED
value: "{{ .Values.hostpathRequiresPrivileged }}"
{{- end }}
{{- if .Values.customHostnameLabel }}
- name: ROOK_CUSTOM_HOSTNAME_LABEL
value: {{ .Values.customHostnameLabel }}
{{- end }}
- name: ROOK_DISABLE_DEVICE_HOTPLUG
value: "{{ .Values.disableDeviceHotplug }}"
Expand Down
3 changes: 3 additions & 0 deletions deploy/charts/rook-ceph/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,9 @@ discover:
# cpu: 100m
# memory: 128Mi

# -- Custom label to identify node hostname. If not set `kubernetes.io/hostname` will be used
customHostnameLabel:

# -- Runs Ceph Pods as privileged to be able to write to `hostPaths` in OpenShift with SELinux restrictions.
hostpathRequiresPrivileged: false

Expand Down
4 changes: 4 additions & 0 deletions deploy/examples/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,10 @@ spec:
- name: ROOK_UNREACHABLE_NODE_TOLERATION_SECONDS
value: "5"

# Custom label to identify node hostname. If not set `kubernetes.io/hostname` will be used
- name: ROOK_CUSTOM_HOSTNAME_LABEL
value: ""

# The name of the node to pass with the downward API
- name: NODE_NAME
valueFrom:
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/ceph/cluster/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (c *ClusterController) startCleanUpJobs(cluster *cephv1.CephCluster, cephHo
logger.Infof("starting clean up job on node %q", hostName)
jobName := k8sutil.TruncateNodeNameForJob("cluster-cleanup-job-%s", hostName)
podSpec := c.cleanUpJobTemplateSpec(cluster, monSecret, clusterFSID)
podSpec.Spec.NodeSelector = map[string]string{v1.LabelHostname: hostName}
podSpec.Spec.NodeSelector = map[string]string{k8sutil.LabelHostname(): hostName}
labels := controller.AppLabels(CleanupAppName, cluster.Namespace)
labels[CleanupAppName] = "true"
job := &batch.Job{
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/ceph/cluster/mgr/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (c *Cluster) makeDeployment(mgrConfig *mgrConfig) (*apps.Deployment, error)
mon.CephSecretVolume())

// Stretch the mgrs across hosts by default, or across a bigger failure domain for when zones are required like in case of stretched cluster
topologyKey := v1.LabelHostname
topologyKey := k8sutil.LabelHostname()
if c.spec.ZonesRequired() {
topologyKey = mon.GetFailureDomainLabel(c.spec)
}
Expand Down
17 changes: 11 additions & 6 deletions pkg/operator/ceph/cluster/mon/mon.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ func scheduleMonitor(c *Cluster, mon *monConfig) (*apps.Deployment, error) {
// setup affinity settings for pod scheduling
p := c.getMonPlacement(mon.Zone)
p.ApplyToPodSpec(&d.Spec.Template.Spec)
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), v1.LabelHostname,
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), k8sutil.LabelHostname(),
map[string]string{k8sutil.AppAttr: AppName}, nil)

// setup storage on the canary since scheduling will be affected when
Expand Down Expand Up @@ -1412,10 +1412,15 @@ func (c *Cluster) startMon(m *monConfig, schedule *controller.MonScheduleInfo) e
if m.UseHostNetwork || !pvcExists {
p.PodAffinity = nil
p.PodAntiAffinity = nil
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), v1.LabelHostname,
map[string]string{k8sutil.AppAttr: AppName}, existingDeployment.Spec.Template.Spec.NodeSelector)
nodeSelector := existingDeployment.Spec.Template.Spec.NodeSelector
if schedule != nil && schedule.Hostname != "" {
// update nodeSelector in case if ROOK_CUSTOM_HOSTNAME_LABEL was changed:
nodeSelector = map[string]string{k8sutil.LabelHostname(): schedule.Hostname}
}
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), k8sutil.LabelHostname(),
map[string]string{k8sutil.AppAttr: AppName}, nodeSelector)
} else {
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), v1.LabelHostname,
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), k8sutil.LabelHostname(),
map[string]string{k8sutil.AppAttr: AppName}, nil)
}
return c.updateMon(m, d)
Expand Down Expand Up @@ -1445,9 +1450,9 @@ func (c *Cluster) startMon(m *monConfig, schedule *controller.MonScheduleInfo) e
// Schedule the mon on a specific host if specified, or else allow it to be portable according to the PV
p.PodAffinity = nil
p.PodAntiAffinity = nil
nodeSelector = map[string]string{v1.LabelHostname: schedule.Hostname}
nodeSelector = map[string]string{k8sutil.LabelHostname(): schedule.Hostname}
}
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), v1.LabelHostname,
k8sutil.SetNodeAntiAffinityForPod(&d.Spec.Template.Spec, requiredDuringScheduling(&c.spec), k8sutil.LabelHostname(),
map[string]string{k8sutil.AppAttr: AppName}, nodeSelector)

logger.Debugf("Starting mon: %+v", d.Name)
Expand Down
3 changes: 2 additions & 1 deletion pkg/operator/ceph/cluster/mon/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package mon
import (
"github.com/pkg/errors"
opcontroller "github.com/rook/rook/pkg/operator/ceph/controller"
"github.com/rook/rook/pkg/operator/k8sutil"
v1 "k8s.io/api/core/v1"
)

Expand All @@ -29,7 +30,7 @@ const (
func getNodeInfoFromNode(n v1.Node) (*opcontroller.MonScheduleInfo, error) {
nr := &opcontroller.MonScheduleInfo{
Name: n.Name,
Hostname: n.Labels[v1.LabelHostname],
Hostname: n.Labels[k8sutil.LabelHostname()],
}

// If the host networking is setup such that a different IP should be used
Expand Down
18 changes: 9 additions & 9 deletions pkg/operator/ceph/cluster/nodedaemon/crash.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ const (
// createOrUpdateCephCrash is a wrapper around controllerutil.CreateOrUpdate
func (r *ReconcileNode) createOrUpdateCephCrash(node corev1.Node, tolerations []corev1.Toleration, cephCluster cephv1.CephCluster, cephVersion *cephver.CephVersion) (controllerutil.OperationResult, error) {
// Create or Update the deployment default/foo
nodeHostnameLabel, ok := node.ObjectMeta.Labels[corev1.LabelHostname]
nodeHostnameLabel, ok := node.ObjectMeta.Labels[k8sutil.LabelHostname()]
if !ok {
return controllerutil.OperationResultNone, errors.Errorf("label key %q does not exist on node %q", corev1.LabelHostname, node.GetName())
return controllerutil.OperationResultNone, errors.Errorf("label key %q does not exist on node %q", k8sutil.LabelHostname(), node.GetName())
}
deploy := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -66,21 +66,21 @@ func (r *ReconcileNode) createOrUpdateCephCrash(node corev1.Node, tolerations []

// labels for the pod, the deployment, and the deploymentSelector
deploymentLabels := map[string]string{
corev1.LabelHostname: nodeHostnameLabel,
k8sutil.AppAttr: CrashCollectorAppName,
NodeNameLabel: node.GetName(),
k8sutil.LabelHostname(): nodeHostnameLabel,
k8sutil.AppAttr: CrashCollectorAppName,
NodeNameLabel: node.GetName(),
}
deploymentLabels[config.CrashType] = "crash"
deploymentLabels[controller.DaemonIDLabel] = "crash"
deploymentLabels[k8sutil.ClusterAttr] = cephCluster.GetNamespace()

selectorLabels := map[string]string{
corev1.LabelHostname: nodeHostnameLabel,
k8sutil.AppAttr: CrashCollectorAppName,
NodeNameLabel: node.GetName(),
k8sutil.LabelHostname(): nodeHostnameLabel,
k8sutil.AppAttr: CrashCollectorAppName,
NodeNameLabel: node.GetName(),
}

nodeSelector := map[string]string{corev1.LabelHostname: nodeHostnameLabel}
nodeSelector := map[string]string{k8sutil.LabelHostname(): nodeHostnameLabel}

// Deployment selector is immutable so we set this value only if
// a new object is going to be created
Expand Down
18 changes: 9 additions & 9 deletions pkg/operator/ceph/cluster/nodedaemon/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ func (r *ReconcileNode) createOrUpdateCephExporter(node corev1.Node, tolerations
return controllerutil.OperationResultNone, nil
}

nodeHostnameLabel, ok := node.Labels[corev1.LabelHostname]
nodeHostnameLabel, ok := node.Labels[k8sutil.LabelHostname()]
if !ok {
return controllerutil.OperationResultNone, errors.Errorf("label key %q does not exist on node %q", corev1.LabelHostname, node.GetName())
return controllerutil.OperationResultNone, errors.Errorf("label key %q does not exist on node %q", k8sutil.LabelHostname(), node.GetName())
}
deploy := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -91,20 +91,20 @@ func (r *ReconcileNode) createOrUpdateCephExporter(node corev1.Node, tolerations

// labels for the pod, the deployment, and the deploymentSelector
deploymentLabels := map[string]string{
corev1.LabelHostname: nodeHostnameLabel,
k8sutil.AppAttr: cephExporterAppName,
NodeNameLabel: node.GetName(),
k8sutil.LabelHostname(): nodeHostnameLabel,
k8sutil.AppAttr: cephExporterAppName,
NodeNameLabel: node.GetName(),
}
deploymentLabels[controller.DaemonIDLabel] = "exporter"
deploymentLabels[k8sutil.ClusterAttr] = cephCluster.GetNamespace()

selectorLabels := map[string]string{
corev1.LabelHostname: nodeHostnameLabel,
k8sutil.AppAttr: cephExporterAppName,
NodeNameLabel: node.GetName(),
k8sutil.LabelHostname(): nodeHostnameLabel,
k8sutil.AppAttr: cephExporterAppName,
NodeNameLabel: node.GetName(),
}

nodeSelector := map[string]string{corev1.LabelHostname: nodeHostnameLabel}
nodeSelector := map[string]string{k8sutil.LabelHostname(): nodeHostnameLabel}

// Deployment selector is immutable so we set this value only if
// a new object is going to be created
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/ceph/cluster/osd/key_rotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func applyKeyRotationPlacement(spec *v1.PodSpec, labels map[string]string) {
LabelSelector: &metav1.LabelSelector{
MatchLabels: labels,
},
TopologyKey: v1.LabelHostname,
TopologyKey: k8sutil.LabelHostname(),
},
},
}
Expand Down
14 changes: 11 additions & 3 deletions pkg/operator/ceph/cluster/osd/osd.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ func (c *Cluster) getPVCHostName(pvcName string) (string, error) {
for _, d := range deployments.Items {
selectors := d.Spec.Template.Spec.NodeSelector
for label, value := range selectors {
if label == corev1.LabelHostname {
if label == k8sutil.LabelHostname() {
return value, nil
}
}
Expand Down Expand Up @@ -734,10 +734,18 @@ func getNodeOrPVCName(d *appsv1.Deployment) (string, error) {
return v, nil // OSD is on PVC
}
for k, v := range d.Spec.Template.Spec.NodeSelector {
if k == corev1.LabelHostname {
if k == k8sutil.LabelHostname() {
return v, nil
}
}
// try to fallback on previous hostname label
// NodeSelector always has a single entry
if len(d.Spec.Template.Spec.NodeSelector) == 1 {
for _, v := range d.Spec.Template.Spec.NodeSelector {
return v, nil
}
}

return "", errors.Errorf("failed to find node/PVC name for OSD deployment %q: %+v", d.Name, d)
}

Expand Down Expand Up @@ -858,7 +866,7 @@ func getNode(ctx context.Context, clientset kubernetes.Interface, nodeName strin
// try to find by the node by matching the provided nodeName
node, err = clientset.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{})
if kerrors.IsNotFound(err) {
listOpts := metav1.ListOptions{LabelSelector: fmt.Sprintf("%q=%q", corev1.LabelHostname, nodeName)}
listOpts := metav1.ListOptions{LabelSelector: fmt.Sprintf("%q=%q", k8sutil.LabelHostname(), nodeName)}
nodeList, err := clientset.CoreV1().Nodes().List(ctx, listOpts)
if err != nil || len(nodeList.Items) < 1 {
return nil, errors.Wrapf(err, "could not find node %q hostname label", nodeName)
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/ceph/cluster/osd/provision_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (c *Cluster) makeJob(osdProps osdProperties, provisionConfig *provisionConf
podSpec.Spec.InitContainers = append(podSpec.Spec.InitContainers, c.getPVCWalInitContainer("/wal", osdProps))
}
} else {
podSpec.Spec.NodeSelector = map[string]string{v1.LabelHostname: osdProps.crushHostname}
podSpec.Spec.NodeSelector = map[string]string{k8sutil.LabelHostname(): osdProps.crushHostname}
}

job := &batch.Job{
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/ceph/cluster/osd/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd *OSDInfo, provision
return nil, err
}
} else {
deployment.Spec.Template.Spec.NodeSelector = map[string]string{v1.LabelHostname: osdProps.crushHostname}
deployment.Spec.Template.Spec.NodeSelector = map[string]string{k8sutil.LabelHostname(): osdProps.crushHostname}
}
k8sutil.AddRookVersionLabelToDeployment(deployment)
cephv1.GetOSDAnnotations(c.spec.Annotations).ApplyToObjectMeta(&deployment.ObjectMeta)
Expand Down
27 changes: 23 additions & 4 deletions pkg/operator/ceph/cluster/osd/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,18 @@ func TestPodContainer(t *testing.T) {
}

func TestDaemonset(t *testing.T) {
testPodDevices(t, "", "sda", true)
testPodDevices(t, "/var/lib/mydatadir", "sdb", false)
testPodDevices(t, "", "", true)
testPodDevices(t, "", "", false)
t.Run(("device name and all devices"), func(t *testing.T) {
testPodDevices(t, "", "sda", true)
})
t.Run(("data dir and device name"), func(t *testing.T) {
testPodDevices(t, "/var/lib/mydatadir", "sdb", false)
})
t.Run(("all devices"), func(t *testing.T) {
testPodDevices(t, "", "", true)
})
t.Run(("no data dir and device name"), func(t *testing.T) {
testPodDevices(t, "", "", false)
})
}

func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) {
Expand Down Expand Up @@ -493,6 +501,17 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) {
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)
})

// test custom topology label
t.Setenv("ROOK_CUSTOM_HOSTNAME_LABEL", "my_custom_hostname_label")
deployment, err = c.makeDeployment(osdProp, osd, dataPathMap)
assert.Nil(t, err)
assert.NotNil(t, deployment)
assert.Equal(t, "rook-ceph-osd-0", deployment.Name)
assert.Equal(t, c.clusterInfo.Namespace, deployment.Namespace)
assert.Equal(t, serviceAccountName, deployment.Spec.Template.Spec.ServiceAccountName)
assert.Equal(t, int32(1), *(deployment.Spec.Replicas))
assert.Equal(t, "node1", deployment.Spec.Template.Spec.NodeSelector["my_custom_hostname_label"])
}

func verifyEnvVar(t *testing.T, envVars []corev1.EnvVar, expectedName, expectedValue string, expectedFound bool) {
Expand Down
11 changes: 5 additions & 6 deletions pkg/operator/ceph/cluster/osd/topology/topology.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/coreos/pkg/capnslog"
"github.com/rook/rook/pkg/daemon/ceph/client"
"github.com/rook/rook/pkg/operator/k8sutil"
corev1 "k8s.io/api/core/v1"
)

Expand All @@ -46,7 +47,6 @@ var (

const (
topologyLabelPrefix = "topology.rook.io/"
labelHostname = "kubernetes.io/hostname"
)

// ExtractOSDTopologyFromLabels extracts rook topology from labels and returns a map from topology type to value
Expand Down Expand Up @@ -74,16 +74,15 @@ func allKubernetesTopologyLabelsOrdered() []string {
append([]string{corev1.LabelTopologyRegion,
corev1.LabelTopologyZone},
rookTopologyLabelsOrdered()...),
labelHostname, // host is the lowest level in the crush map hierarchy
k8sutil.LabelHostname(), // host is the lowest level in the crush map hierarchy
)
}

func kubernetesTopologyLabelToCRUSHLabel(label string) string {
crushLabel := strings.Split(label, "/")
if crushLabel[len(crushLabel)-1] == "hostname" {
// kubernetes uses "kubernetes.io/hostname" whereas CRUSH uses "host"
if label == k8sutil.LabelHostname() {
return "host"
}
crushLabel := strings.Split(label, "/")
return crushLabel[len(crushLabel)-1]
}

Expand Down Expand Up @@ -140,7 +139,7 @@ func formatTopologyAffinity(label, value string) string {

// GetDefaultTopologyLabels returns the supported default topology labels.
func GetDefaultTopologyLabels() string {
Labels := []string{corev1.LabelHostname, corev1.LabelZoneRegionStable, corev1.LabelZoneFailureDomainStable}
Labels := []string{k8sutil.LabelHostname(), corev1.LabelZoneRegionStable, corev1.LabelZoneFailureDomainStable}
for _, label := range CRUSHTopologyLabels {
Labels = append(Labels, topologyLabelPrefix+label)
}
Expand Down
Loading

0 comments on commit 3112a2b

Please sign in to comment.