From 795124b7a8d625df7726679941e4f9a18adf47b0 Mon Sep 17 00:00:00 2001 From: Blaine Gardner Date: Tue, 2 Mar 2021 16:06:34 -0700 Subject: [PATCH] ceph: update osds in parallel Update OSDs in parallel per the design in design/ceph/update-osds-in-parallel.md The max number of OSDs updated in parallel is currently fixed at 20. Signed-off-by: Blaine Gardner --- PendingReleaseNotes.md | 5 +- .../ceph/csi/rbd/storageclass-test.yaml | 2 +- design/ceph/update-osds-in-parallel.md | 19 +- pkg/apis/rook.io/v1/storage.go | 10 + pkg/apis/rook.io/v1/storage_test.go | 32 + pkg/daemon/ceph/client/fake/osd.go | 130 ++++ pkg/daemon/ceph/client/osd.go | 50 ++ pkg/daemon/ceph/client/osd_test.go | 89 +++ pkg/daemon/ceph/client/upgrade.go | 35 +- pkg/daemon/ceph/client/upgrade_test.go | 61 ++ pkg/daemon/ceph/osd/volume.go | 4 +- pkg/operator/ceph/cluster/osd/create.go | 413 ++++++++++ pkg/operator/ceph/cluster/osd/create_test.go | 705 +++++++++++++++++ pkg/operator/ceph/cluster/osd/deviceSet.go | 34 +- .../ceph/cluster/osd/deviceset_test.go | 12 +- pkg/operator/ceph/cluster/osd/envs.go | 4 + pkg/operator/ceph/cluster/osd/health.go | 2 +- .../ceph/cluster/osd/integration_test.go | 679 +++++++++++++++++ pkg/operator/ceph/cluster/osd/osd.go | 581 +++++--------- pkg/operator/ceph/cluster/osd/osd_pvc_test.go | 680 ----------------- pkg/operator/ceph/cluster/osd/osd_test.go | 216 ++++-- pkg/operator/ceph/cluster/osd/spec.go | 27 +- pkg/operator/ceph/cluster/osd/spec_test.go | 50 +- pkg/operator/ceph/cluster/osd/status.go | 521 ++++++------- pkg/operator/ceph/cluster/osd/status_test.go | 12 +- pkg/operator/ceph/cluster/osd/update.go | 339 +++++++++ pkg/operator/ceph/cluster/osd/update_test.go | 713 ++++++++++++++++++ pkg/operator/ceph/cluster/predicate.go | 14 +- pkg/operator/ceph/controller/spec.go | 2 +- pkg/operator/ceph/csi/spec.go | 4 +- pkg/operator/ceph/webhook.go | 2 +- pkg/operator/k8sutil/daemonset.go | 9 +- pkg/operator/k8sutil/deployment.go | 293 ++++++- pkg/operator/k8sutil/deployment_test.go | 482 ++++++++++++ pkg/operator/k8sutil/job.go | 2 +- pkg/operator/k8sutil/k8sutil.go | 12 + pkg/operator/k8sutil/prometheus.go | 7 +- pkg/operator/test/client.go | 29 +- pkg/util/retry.go | 45 ++ pkg/util/set.go | 2 +- 40 files changed, 4769 insertions(+), 1559 deletions(-) create mode 100644 pkg/daemon/ceph/client/fake/osd.go create mode 100644 pkg/operator/ceph/cluster/osd/create.go create mode 100644 pkg/operator/ceph/cluster/osd/create_test.go create mode 100644 pkg/operator/ceph/cluster/osd/integration_test.go delete mode 100644 pkg/operator/ceph/cluster/osd/osd_pvc_test.go create mode 100644 pkg/operator/ceph/cluster/osd/update.go create mode 100644 pkg/operator/ceph/cluster/osd/update_test.go create mode 100644 pkg/operator/k8sutil/deployment_test.go diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index fa407a7a2..fe14ba12c 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -27,5 +27,8 @@ v1.6... * Enable disruption budgets (PDBs) by default for Mon, RGW, MDS, and OSD daemons * Add CephFilesystemMirror CRD to deploy cephfs-mirror daemon * Multiple Ceph mgr daemons are supported for stretch clusters and other clusters where HA of the mgr is more critical -* Ceph OSD: as of Nautilus 14.2.14 and Octopus 15.2.9 if the OSD scenario is simple (one OSD per disk) we won't use LVM to prepare the disk anymore +* OSDs: + * as of Nautilus 14.2.14 and Octopus 15.2.9 if the OSD scenario is simple (one OSD per disk) we won't use LVM to prepare the disk anymore + * for Pacific (16.2.x), Rook is able to update multiple OSD Deployments at the same time to speed + up updates and upgrades for larger Ceph clusters * Disable CSI GRPC metrics by default diff --git a/cluster/examples/kubernetes/ceph/csi/rbd/storageclass-test.yaml b/cluster/examples/kubernetes/ceph/csi/rbd/storageclass-test.yaml index fbe52e3f3..f5cf3b45c 100644 --- a/cluster/examples/kubernetes/ceph/csi/rbd/storageclass-test.yaml +++ b/cluster/examples/kubernetes/ceph/csi/rbd/storageclass-test.yaml @@ -4,7 +4,7 @@ metadata: name: replicapool namespace: rook-ceph # namespace:cluster spec: - failureDomain: host + failureDomain: osd replicated: size: 1 # Disallow setting pool with replica 1, this could lead to data loss without recovery. diff --git a/design/ceph/update-osds-in-parallel.md b/design/ceph/update-osds-in-parallel.md index 265221caf..2cc4f8cac 100644 --- a/design/ceph/update-osds-in-parallel.md +++ b/design/ceph/update-osds-in-parallel.md @@ -97,18 +97,21 @@ List all OSD Deployments belonging to the Rook cluster. Build a list of OSD IDs Deployments. Record this in a data structure that allows O(1) lookup. ### How to build the update queue -List all OSD Deployments belonging to the Rook cluster to use as the update queue. Ignore OSD -Deployments which are already updated. OSD Deployments which need updated will have a -`rook-version` label that does not match the current version of the Rook operator and a -`ceph-version` label that does not match the current Ceph version being deployed. - -Go channels are likely not the best option for the update queue since it may be necessary to select -individual items from the queue at any position and not merely grab the top/bottom item from the -queue. +List all OSD Deployments belonging to the Rook cluster to use as the update queue. All OSDs should +be updated in case there are changes to the CephCluster resource that result in OSD deployments +being updated. The minimal information each item in the queue needs is only the OSD ID. The OSD Deployment managed by Rook can easily be inferred from the OSD ID. +Note: A previous version of this design planned to ignore OSD Deployments which are already updated. +The plan was to identify OSD Deployments which need updated by looking at the OSD Deployments for: +(1) a `rook-version` label that does not match the current version of the Rook operator AND/OR +(2) a `ceph-version` label that does not match the current Ceph version being deployed. This is an +invalid optimization that does not account for OSD Deployments changing due to CephCluster resource +updates. Instead of trying to optimize, it is better to always update OSD Deplyments and rely on the +lower level update calls to finish quickly when there is no update to apply. + ### User configuration #### Modifications to `CephCluster` CRD diff --git a/pkg/apis/rook.io/v1/storage.go b/pkg/apis/rook.io/v1/storage.go index 53f9ca00e..1f5cd9c71 100644 --- a/pkg/apis/rook.io/v1/storage.go +++ b/pkg/apis/rook.io/v1/storage.go @@ -39,6 +39,16 @@ func (s *StorageScopeSpec) ClearUseAllDevices() { } } +// NodeExists returns true if the node exists in the storage spec. False otherwise. +func (s *StorageScopeSpec) NodeExists(nodeName string) bool { + for i := range s.Nodes { + if s.Nodes[i].Name == nodeName { + return true + } + } + return false +} + // Fully resolves the config of the given node name, taking into account cluster level and node level specified config. // In general, the more fine grained the configuration is specified, the more precedence it takes. Fully resolved // configuration for the node has the following order of precedence. diff --git a/pkg/apis/rook.io/v1/storage_test.go b/pkg/apis/rook.io/v1/storage_test.go index cf1dec9e0..561fc96fb 100644 --- a/pkg/apis/rook.io/v1/storage_test.go +++ b/pkg/apis/rook.io/v1/storage_test.go @@ -21,6 +21,38 @@ import ( "github.com/stretchr/testify/assert" ) +func TestNodeExists(t *testing.T) { + t.Run("does not exist - no nodes specified", func(t *testing.T) { + spec := StorageScopeSpec{} + assert.False(t, spec.NodeExists("does-not-exist")) + }) + + t.Run("exists - single node specified", func(t *testing.T) { + spec := StorageScopeSpec{ + Nodes: []Node{ + {Name: "node1"}, // node gets nothing but its name set + }, + } + assert.True(t, spec.NodeExists("node1")) + }) + + t.Run("exists and not exists - multiple nodes specified", func(t *testing.T) { + spec := StorageScopeSpec{ + Nodes: []Node{ + {Name: "node1"}, // node gets nothing but its name set + {Name: "node3"}, + {Name: "node4"}, + }, + } + assert.True(t, spec.NodeExists("node1")) + assert.False(t, spec.NodeExists("node2")) + assert.True(t, spec.NodeExists("node3")) + assert.True(t, spec.NodeExists("node4")) + assert.False(t, spec.NodeExists("node5")) + assert.False(t, spec.NodeExists("does-not-exist")) + }) +} + func TestResolveNodeNotExist(t *testing.T) { // a non existing node should return nil storageSpec := StorageScopeSpec{} diff --git a/pkg/daemon/ceph/client/fake/osd.go b/pkg/daemon/ceph/client/fake/osd.go new file mode 100644 index 000000000..c1d4a0b52 --- /dev/null +++ b/pkg/daemon/ceph/client/fake/osd.go @@ -0,0 +1,130 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fake + +import ( + "fmt" + "strconv" + "strings" +) + +// OsdLsOutput returns JSON output from 'ceph osd ls' that can be used for unit tests. It +// returns output for a Ceph cluster with the number of OSDs given as input starting with ID 0. +// example: numOSDs = 5 => return: "[0,1,2,3,4]" +func OsdLsOutput(numOSDs int) string { + stringIDs := make([]string, 0, numOSDs) + for id := 0; id < numOSDs; id++ { + stringIDs = append(stringIDs, strconv.Itoa(id)) + } + return fmt.Sprintf("[%s]", strings.Join(stringIDs, ",")) +} + +// OsdTreeOutput returns JSON output from 'ceph osd tree' that can be used for unit tests. +// It returns output for a Ceph cluster with the given number of nodes and the given number of OSDs +// per node with no complex configuration. This should work even for 0 nodes. +// example: OsdTreeOutput(3, 3) // returns JSON output for the Ceph cluster below +// node0: node1: node2: +// - osd0 - osd1 - osd2 +// - osd3 - osd4 - osd5 +// - osd6 - osd7 - osd8 +func OsdTreeOutput(numNodes, numOSDsPerNode int) string { + // JSON output taken from Ceph Pacific + rootFormat := ` { + "id": -1, + "name": "default", + "type": "root", + "type_id": 11, + "children": [%s] + }` // format: negative node IDs as comma-delimited string (e.g., "-3,-4,-5") + nodeFormat := ` { + "id": %d, + "name": "%s", + "type": "host", + "type_id": 1, + "pool_weights": {}, + "children": [%s] + }` // format: negative node ID, node name, OSD IDs as comma-delimited string (e.g., "0,3,6") + osdFormat := ` { + "id": %d, + "device_class": "hdd", + "name": "osd.%d", + "type": "osd", + "type_id": 0, + "crush_weight": 0.009796142578125, + "depth": 2, + "pool_weights": {}, + "exists": 1, + "status": "up", + "reweight": 1, + "primary_affinity": 1 + }` // format: OSD ID, OSD ID + wrapperFormat := `{ + "nodes": [ +%s + ], + "stray": [] +}` // format: + nodesJSON := []string{} + osdsJSON := []string{} + nodes := []string{} + for n := 0; n < numNodes; n++ { + osds := []string{} + nodeName := fmt.Sprintf("node%d", n) + nodeID := -3 - n + nodes = append(nodes, strconv.Itoa(nodeID)) + for i := 0; i < numOSDsPerNode; i++ { + osdID := n + 3*i + osds = append(osds, strconv.Itoa(osdID)) + osdsJSON = append(osdsJSON, fmt.Sprintf(osdFormat, osdID, osdID)) + } + nodesJSON = append(nodesJSON, fmt.Sprintf(nodeFormat, nodeID, nodeName, strings.Join(osds, ","))) + } + rootJSON := fmt.Sprintf(rootFormat, strings.Join(nodes, ",")) + fullJSON := append(append([]string{rootJSON}, nodesJSON...), osdsJSON...) + rendered := fmt.Sprintf(wrapperFormat, strings.Join(fullJSON, ",\n")) + return rendered +} + +// OsdOkToStopOutput returns JSON output from 'ceph osd ok-to-stop' that can be used for unit tests. +// queriedID should be given as the ID sent to the 'osd ok-to-stop [--max=N]' command. It will +// be returned with relevant NOT ok-to-stop results. +// If returnOsdIds is empty, this returns a NOT ok-to-stop result. Otherwise, it returns an +// ok-to-stop result. returnOsdIds should include queriedID if the result should be successful. +// usePacificPlusOutput instructs the function to render output for Ceph Pacific (v16) and above or +// to render output for Ceph Octopus (v15) and below. +func OsdOkToStopOutput(queriedID int, returnOsdIds []int, useCephPacificPlusOutput bool) string { + // For Pacific and up (Pacific+) + okTemplate := `{"ok_to_stop":true,"osds":[%s],"num_ok_pgs":132,"num_not_ok_pgs":0,"ok_become_degraded":["1.0","1.2","1.3"]}` + notOkTemplate := `{"ok_to_stop":false,"osds":[%d],"num_ok_pgs":161,"num_not_ok_pgs":50,"bad_become_inactive":["1.0","1.3","1.a"],"ok_become_degraded":["1.2","1.4","1.5"]}` + + // Ceph Octopus and below don't return anything on stdout, only success/failure via retcode + if !useCephPacificPlusOutput { + return "" + } + + // Pacific+, NOT ok-to-stop + if len(returnOsdIds) == 0 { + return fmt.Sprintf(notOkTemplate, queriedID) + } + + // Pacific+, ok-to-stop + osdIdsStr := make([]string, len(returnOsdIds)) + for i := 0; i < len(returnOsdIds); i++ { + osdIdsStr[i] = strconv.Itoa(returnOsdIds[i]) + } + return fmt.Sprintf(okTemplate, strings.Join(osdIdsStr, ",")) +} diff --git a/pkg/daemon/ceph/client/osd.go b/pkg/daemon/ceph/client/osd.go index 9d1c589e1..9d77f67ec 100644 --- a/pkg/daemon/ceph/client/osd.go +++ b/pkg/daemon/ceph/client/osd.go @@ -17,6 +17,7 @@ package client import ( "encoding/json" + "fmt" "strconv" "strings" @@ -305,3 +306,52 @@ func OsdListNum(context *clusterd.Context, clusterInfo *ClusterInfo) (OsdList, e return output, nil } + +// OSDOkToStopStats report detailed information about which OSDs are okay to stop +type OSDOkToStopStats struct { + OkToStop bool `json:"ok_to_stop"` + OSDs []int `json:"osds"` + NumOkPGs int `json:"num_ok_pgs"` + NumNotOkPGs int `json:"num_not_ok_pgs"` + BadBecomeInactive []string `json:"bad_become_inactive"` + OkBecomeDegraded []string `json:"ok_become_degraded"` +} + +// OSDOkToStop returns a list of OSDs that can be stopped that includes the OSD ID given. +// This is relevant, for example, when checking which OSDs can be updated. +// The number of OSDs returned is limited by the value set in maxReturned. +// maxReturned=0 is the same as maxReturned=1. +func OSDOkToStop(context *clusterd.Context, clusterInfo *ClusterInfo, osdID, maxReturned int) ([]int, error) { + args := []string{"osd", "ok-to-stop", strconv.Itoa(osdID)} + returnsList := false // does the ceph call return a list of OSD IDs? + if clusterInfo.CephVersion.IsAtLeastPacific() { + returnsList = true + // NOTE: if the number of OSD IDs given in the CLI arg query is Q and --max=N is given, if + // N < Q, Ceph treats the query as though max=Q instead, always returning at least Q OSDs. + args = append(args, fmt.Sprintf("--max=%d", maxReturned)) + } + + buf, err := NewCephCommand(context, clusterInfo, args).Run() + if err != nil { + // is not ok to stop (or command error) + return []int{}, errors.Wrapf(err, "OSD %d is not ok to stop", osdID) + } + + if !returnsList { + // If does not return list, just return a slice including only the OSD ID queried + return []int{osdID}, nil + } + + var stats OSDOkToStopStats + err = json.Unmarshal(buf, &stats) + if err != nil { + // Since the command succeeded we still know that at least the given OSD ID is ok to + // stop, so we do not *have* to return an error. However, it is good to do it anyway so + // that we can catch breaking changes to JSON output in CI testing. As a middle ground + // here, return error but also return the given OSD ID in the output in case the calling + // function wants to recover from this case. + return []int{osdID}, errors.Wrapf(err, "failed to unmarshal 'osd ok-to-stop %d' response", osdID) + } + + return stats.OSDs, nil +} diff --git a/pkg/daemon/ceph/client/osd_test.go b/pkg/daemon/ceph/client/osd_test.go index c5b382579..16995df19 100644 --- a/pkg/daemon/ceph/client/osd_test.go +++ b/pkg/daemon/ceph/client/osd_test.go @@ -17,10 +17,13 @@ limitations under the License. package client import ( + "fmt" "testing" "github.com/pkg/errors" "github.com/rook/rook/pkg/clusterd" + "github.com/rook/rook/pkg/daemon/ceph/client/fake" + "github.com/rook/rook/pkg/operator/ceph/version" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" ) @@ -110,3 +113,89 @@ func TestOsdListNum(t *testing.T) { assert.Error(t, err) assert.Equal(t, 0, len(list)) } + +func TestOSDOkToStop(t *testing.T) { + returnString := "" + returnOkResult := true + seenArgs := []string{} + + executor := &exectest.MockExecutor{} + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { + logger.Infof("Command: %s %v", command, args) + switch { + case args[0] == "osd" && args[1] == "ok-to-stop": + seenArgs = args + if returnOkResult { + return returnString, nil + } + return returnString, errors.Errorf("Error EBUSY: unsafe to stop osd(s) at this time (50 PGs are or would become offline)") + } + panic(fmt.Sprintf("unexpected ceph command %q", args)) + } + + context := &clusterd.Context{Executor: executor} + clusterInfo := AdminClusterInfo("mycluster") + + doSetup := func() { + seenArgs = []string{} + } + + t.Run("pacific output ok to stop", func(t *testing.T) { + doSetup() + clusterInfo.CephVersion = version.Pacific + returnString = fake.OsdOkToStopOutput(1, []int{1, 2}, true) + returnOkResult = true + osds, err := OSDOkToStop(context, clusterInfo, 1, 2) + assert.NoError(t, err) + assert.ElementsMatch(t, osds, []int{1, 2}) + assert.Equal(t, "1", seenArgs[2]) + assert.Equal(t, "--max=2", seenArgs[3]) + }) + + t.Run("pacific output not ok to stop", func(t *testing.T) { + doSetup() + clusterInfo.CephVersion = version.Pacific + returnString = fake.OsdOkToStopOutput(3, []int{}, true) + returnOkResult = false + _, err := OSDOkToStop(context, clusterInfo, 3, 5) + assert.Error(t, err) + assert.Equal(t, "3", seenArgs[2]) + assert.Equal(t, "--max=5", seenArgs[3]) + }) + + t.Run("pacific handles maxReturned=0", func(t *testing.T) { + doSetup() + clusterInfo.CephVersion = version.Pacific + returnString = fake.OsdOkToStopOutput(4, []int{4, 8}, true) + returnOkResult = true + osds, err := OSDOkToStop(context, clusterInfo, 4, 0) + assert.NoError(t, err) + assert.ElementsMatch(t, osds, []int{4, 8}) + assert.Equal(t, "4", seenArgs[2]) + // should just pass through as --max=0; don't do any special processing + assert.Equal(t, "--max=0", seenArgs[3]) + }) + + t.Run("octopus output not ok to stop", func(t *testing.T) { + doSetup() + clusterInfo.CephVersion = version.Octopus + returnString = fake.OsdOkToStopOutput(3, []int{}, false) + returnOkResult = false + _, err := OSDOkToStop(context, clusterInfo, 3, 5) + assert.Error(t, err) + assert.Equal(t, "3", seenArgs[2]) + assert.NotContains(t, seenArgs[3], "--max") // do not issue the "--max" flag below pacific + }) + + t.Run("octopus output ok to stop", func(t *testing.T) { + doSetup() + clusterInfo.CephVersion = version.Octopus + returnString = fake.OsdOkToStopOutput(50, []int{50}, false) + returnOkResult = true + osds, err := OSDOkToStop(context, clusterInfo, 50, 2) + assert.NoError(t, err) + assert.ElementsMatch(t, osds, []int{50}) + assert.Equal(t, "50", seenArgs[2]) + assert.NotContains(t, seenArgs[3], "--max") // do not issue the "--max" flag below pacific + }) +} diff --git a/pkg/daemon/ceph/client/upgrade.go b/pkg/daemon/ceph/client/upgrade.go index fcef5dce5..de45de1f5 100644 --- a/pkg/daemon/ceph/client/upgrade.go +++ b/pkg/daemon/ceph/client/upgrade.go @@ -356,6 +356,40 @@ func buildHostListFromTree(tree OsdTree) (OsdTree, error) { return osdList, nil } +// OSDUpdateShouldCheckOkToStop returns true if Rook should check ok-to-stop for OSDs when doing +// OSD daemon updates. It will return false if it should not perform ok-to-stop checks, for example, +// when there are fewer than 3 OSDs +func OSDUpdateShouldCheckOkToStop(context *clusterd.Context, clusterInfo *ClusterInfo) bool { + userIntervention := "the user will likely need to set continueUpgradeAfterChecksEvenIfNotHealthy to allow OSD updates to proceed" + + osds, err := OsdListNum(context, clusterInfo) + if err != nil { + // If calling osd list fails, we assume there are more than 3 OSDs and we check if ok-to-stop + // If there are less than 3 OSDs, the ok-to-stop call will fail + // this can still be controlled by setting continueUpgradeAfterChecksEvenIfNotHealthy + // At least this will happen for a single OSD only, which means 2 OSDs will restart in a small interval + logger.Warningf("failed to determine the total number of osds. will check if OSDs are ok-to-stop. if there are fewer than 3 OSDs %s. %v", userIntervention, err) + return true + } + if len(osds) < 3 { + logger.Warningf("the cluster has fewer than 3 osds. not performing upgrade check. running in best-effort") + return false + } + + // aio means all in one + aio, err := allOSDsSameHost(context, clusterInfo) + if err != nil { + logger.Warningf("failed to determine if all osds are running on the same host. will check if OSDs are ok-to-stop. if all OSDs are running on one host %s. %v", userIntervention, err) + return true + } + if aio { + logger.Warningf("all OSDs are running on the same host. not performing upgrade check. running in best-effort") + return false + } + + return true +} + // osdDoNothing determines whether we should perform upgrade pre-check and post-checks for the OSD daemon // it checks for various cluster info like number of OSD and their placement // it returns 'true' if we need to do nothing and false and we should pre-check/post-check @@ -377,7 +411,6 @@ func osdDoNothing(context *clusterd.Context, clusterInfo *ClusterInfo) bool { // aio means all in one aio, err := allOSDsSameHost(context, clusterInfo) if err != nil { - // If calling osd list fails, we assume there are more than 3 OSDs and we check if ok-to-stop logger.Warningf("failed to determine if all osds are running on the same host, performing upgrade check anyways. %v", err) return false } diff --git a/pkg/daemon/ceph/client/upgrade_test.go b/pkg/daemon/ceph/client/upgrade_test.go index e6aaf1518..5c3db2005 100644 --- a/pkg/daemon/ceph/client/upgrade_test.go +++ b/pkg/daemon/ceph/client/upgrade_test.go @@ -23,6 +23,7 @@ import ( "github.com/pkg/errors" "github.com/rook/rook/pkg/clusterd" + "github.com/rook/rook/pkg/daemon/ceph/client/fake" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" ) @@ -297,3 +298,63 @@ func TestGetRetryConfig(t *testing.T) { assert.Equalf(t, tc.expectedDelay, actualDelay, "[%s] failed to get correct delays between retries", tc.label) } } + +func TestOSDUpdateShouldCheckOkToStop(t *testing.T) { + clusterInfo := &ClusterInfo{} + lsOutput := "" + treeOutput := "" + context := &clusterd.Context{ + Executor: &exectest.MockExecutor{ + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { + t.Logf("command: %s %v", command, args) + if command != "ceph" || args[0] != "osd" { + panic("not a 'ceph osd' call") + } + if args[1] == "tree" { + if treeOutput == "" { + return "", errors.Errorf("induced error") + } + return treeOutput, nil + } + if args[1] == "ls" { + if lsOutput == "" { + return "", errors.Errorf("induced error") + } + return lsOutput, nil + } + panic("do not understand command") + }, + }, + } + + t.Run("3 nodes with 1 OSD each", func(t *testing.T) { + lsOutput = fake.OsdLsOutput(3) + treeOutput = fake.OsdTreeOutput(3, 1) + assert.True(t, OSDUpdateShouldCheckOkToStop(context, clusterInfo)) + }) + + t.Run("1 node with 3 OSDs", func(t *testing.T) { + lsOutput = fake.OsdLsOutput(3) + treeOutput = fake.OsdTreeOutput(1, 3) + assert.False(t, OSDUpdateShouldCheckOkToStop(context, clusterInfo)) + }) + + t.Run("2 nodes with 1 OSD each", func(t *testing.T) { + lsOutput = fake.OsdLsOutput(2) + treeOutput = fake.OsdTreeOutput(2, 1) + assert.False(t, OSDUpdateShouldCheckOkToStop(context, clusterInfo)) + }) + + t.Run("3 nodes with 3 OSDs each", func(t *testing.T) { + lsOutput = fake.OsdLsOutput(9) + treeOutput = fake.OsdTreeOutput(3, 3) + assert.True(t, OSDUpdateShouldCheckOkToStop(context, clusterInfo)) + }) + + // degraded case but good to test just in case + t.Run("0 nodes", func(t *testing.T) { + lsOutput = fake.OsdLsOutput(0) + treeOutput = fake.OsdTreeOutput(0, 0) + assert.False(t, OSDUpdateShouldCheckOkToStop(context, clusterInfo)) + }) +} diff --git a/pkg/daemon/ceph/osd/volume.go b/pkg/daemon/ceph/osd/volume.go index 25b040092..dca8540a9 100644 --- a/pkg/daemon/ceph/osd/volume.go +++ b/pkg/daemon/ceph/osd/volume.go @@ -778,12 +778,12 @@ func (a *OsdAgent) initializeDevicesLVMMode(context *clusterd.Context, devices * } if len(strings.Split(conf["devices"], " ")) != len(cvReports) { - return fmt.Errorf("failed to create enough required devices, required: %s, actual: %v", cvOut, cvReports) + return errors.Errorf("failed to create enough required devices, required: %s, actual: %v", cvOut, cvReports) } for _, report := range cvReports { if report.BlockDB != mdPath { - return fmt.Errorf("wrong db device for %s, required: %s, actual: %s", report.Data, mdPath, report.BlockDB) + return errors.Errorf("wrong db device for %s, required: %s, actual: %s", report.Data, mdPath, report.BlockDB) } } } diff --git a/pkg/operator/ceph/cluster/osd/create.go b/pkg/operator/ceph/cluster/osd/create.go new file mode 100644 index 000000000..97452e10d --- /dev/null +++ b/pkg/operator/ceph/cluster/osd/create.go @@ -0,0 +1,413 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package osd + +import ( + "fmt" + + "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + rookv1 "github.com/rook/rook/pkg/apis/rook.io/v1" + kms "github.com/rook/rook/pkg/daemon/ceph/osd/kms" + osdconfig "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" + "github.com/rook/rook/pkg/operator/ceph/controller" + opcontroller "github.com/rook/rook/pkg/operator/ceph/controller" + "github.com/rook/rook/pkg/operator/k8sutil" + "github.com/rook/rook/pkg/util" + v1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/version" +) + +type createConfig struct { + cluster *Cluster + provisionConfig *provisionConfig + awaitingStatusConfigMaps *util.Set // These status configmaps were created for OSD prepare jobs + finishedStatusConfigMaps *util.Set // Status configmaps are added here as provisioning is completed for them + deployments *existenceList // these OSDs have existing deployments +} + +// allow overriding these functions for unit tests +var ( + createDaemonOnNodeFunc = createDaemonOnNode + createDaemonOnPVCFunc = createDaemonOnPVC + + updateConditionFunc = opcontroller.UpdateCondition +) + +func (c *Cluster) newCreateConfig( + provisionConfig *provisionConfig, + awaitingStatusConfigMaps *util.Set, + deployments *existenceList, +) *createConfig { + if awaitingStatusConfigMaps == nil { + awaitingStatusConfigMaps = util.NewSet() + } + return &createConfig{ + c, + provisionConfig, + awaitingStatusConfigMaps, + util.NewSet(), + deployments, + } +} + +func (c *createConfig) progress() (completed, initial int) { + return c.finishedStatusConfigMaps.Count(), c.awaitingStatusConfigMaps.Count() +} + +func (c *createConfig) doneCreating() bool { + return c.awaitingStatusConfigMaps.Count() == c.finishedStatusConfigMaps.Count() +} + +func (c *createConfig) createNewOSDsFromStatus( + status *OrchestrationStatus, + nodeOrPVCName string, + errs *provisionErrors, +) { + if !c.awaitingStatusConfigMaps.Contains(statusConfigMapName(nodeOrPVCName)) { + // If there is a dangling OSD prepare configmap from another reconcile, don't process it + logger.Infof("not creating deployments for OSD prepare results found in ConfigMap %q which was not created for the latest storage spec", statusConfigMapName(nodeOrPVCName)) + return + } + + if c.finishedStatusConfigMaps.Contains(statusConfigMapName(nodeOrPVCName)) { + // If we have already processed this configmap, don't process it again + logger.Infof("not creating deployments for OSD prepare results found in ConfigMap %q which was already processed", statusConfigMapName(nodeOrPVCName)) + return + } + + for _, osd := range status.OSDs { + if c.deployments.Exists(osd.ID) { + // This OSD will be handled by the updater + logger.Debugf("not creating deployment for OSD %d which already exists", osd.ID) + continue + } + if status.PvcBackedOSD { + logger.Infof("creating OSD %d on PVC %q", osd.ID, nodeOrPVCName) + err := createDaemonOnPVCFunc(c.cluster, osd, nodeOrPVCName, c.provisionConfig) + if err != nil { + errs.addError("%v", errors.Wrapf(err, "failed to create OSD %d on PVC %q", osd.ID, nodeOrPVCName)) + } + } else { + logger.Infof("creating OSD %d on node %q", osd.ID, nodeOrPVCName) + err := createDaemonOnNodeFunc(c.cluster, osd, nodeOrPVCName, c.provisionConfig) + if err != nil { + errs.addError("%v", errors.Wrapf(err, "failed to create OSD %d on node %q", osd.ID, nodeOrPVCName)) + } + } + } + + c.doneWithStatus(nodeOrPVCName) +} + +// Call this if createNewOSDsFromStatus() isn't going to be called (like for a failed status) +func (c *createConfig) doneWithStatus(nodeOrPVCName string) { + c.finishedStatusConfigMaps.Add(statusConfigMapName(nodeOrPVCName)) +} + +// Returns a set of all the awaitingStatusConfigMaps that will be updated by provisioning jobs. +// Returns error only if the calling function should halt all OSD provisioning. Non-halting errors +// are added to provisionErrors. +// +// Creation of prepare jobs is most directly related to creating new OSDs. And we want to keep all +// usage of awaitingStatusConfigMaps in this file. +func (c *Cluster) startProvisioningOverPVCs(config *provisionConfig, errs *provisionErrors) (*util.Set, error) { + // Parsing storageClassDeviceSets and parsing it to volume sources + c.spec.Storage.VolumeSources = append(c.spec.Storage.VolumeSources, c.prepareStorageClassDeviceSets(errs)...) + + c.ValidStorage.VolumeSources = c.spec.Storage.VolumeSources + + // no valid VolumeSource is ready to run an osd + if !c.shouldProvisionOverPVCs() { + logger.Info("no storageClassDeviceSets or volumeSources are defined to configure OSDs on PVCs") + return util.NewSet(), nil + } + + // Check k8s version + k8sVersion, err := k8sutil.GetK8SVersion(c.context.Clientset) + if err != nil { + errs.addError("failed to provision OSDs on PVCs. user has specified storageClassDeviceSets, but the Kubernetes version could not be determined. minimum Kubernetes version required: 1.13.0. %v", err) + return util.NewSet(), nil + } + if !k8sVersion.AtLeast(version.MustParseSemantic("v1.13.0")) { + errs.addError("failed to provision OSDs on PVCs. user has specified storageClassDeviceSets, but the Kubernetes version is not supported. user must update Kubernetes version. minimum Kubernetes version required: 1.13.0. version detected: %s", k8sVersion.String()) + return util.NewSet(), nil + } + + existingDeployments, err := c.getExistingOSDDeploymentsOnPVCs() + if err != nil { + errs.addError("failed to provision OSDs on PVCs. failed to query existing OSD deployments on PVCs. %v", err) + return util.NewSet(), nil + } + + awaitingStatusConfigMaps := util.NewSet() + for _, volume := range c.ValidStorage.VolumeSources { + // Check whether we need to cancel the orchestration + if err := controller.CheckForCancelledOrchestration(c.context); err != nil { + return awaitingStatusConfigMaps, err + } + + dataSource, dataOK := volume.PVCSources[bluestorePVCData] + + // The data PVC template is required. + if !dataOK { + errs.addError("failed to create OSD provisioner for storageClassDeviceSet %q. missing the data template", volume.Name) + continue + } + + metadataSource, metadataOK := volume.PVCSources[bluestorePVCMetadata] + if metadataOK { + logger.Infof("OSD will have its main bluestore block on %q and its metadata device on %q", dataSource.ClaimName, metadataSource.ClaimName) + } else { + logger.Infof("OSD will have its main bluestore block on %q", dataSource.ClaimName) + } + + walSource, walOK := volume.PVCSources[bluestorePVCWal] + if walOK { + logger.Infof("OSD will have its wal device on %q", walSource.ClaimName) + } + + osdProps := osdProperties{ + crushHostname: dataSource.ClaimName, + pvc: dataSource, + metadataPVC: metadataSource, + walPVC: walSource, + resources: volume.Resources, + placement: volume.Placement, + preparePlacement: volume.PreparePlacement, + portable: volume.Portable, + schedulerName: volume.SchedulerName, + encrypted: volume.Encrypted, + deviceSetName: volume.Name, + } + osdProps.storeConfig.DeviceClass = volume.CrushDeviceClass + + if osdProps.encrypted { + // If the deviceSet template has "encrypted" but the Ceph version is not compatible + if !c.isCephVolumeRawModeSupported() { + errMsg := fmt.Sprintf("failed to validate storageClassDeviceSet %q. min required ceph version to support encryption is %q or %q", volume.Name, cephVolumeRawEncryptionModeMinNautilusCephVersion.String(), cephVolumeRawEncryptionModeMinOctopusCephVersion.String()) + errs.addError(errMsg) + continue + } + + // create encryption Kubernetes Secret if the PVC is encrypted + key, err := generateDmCryptKey() + if err != nil { + errMsg := fmt.Sprintf("failed to generate dmcrypt key for osd claim %q. %v", osdProps.pvc.ClaimName, err) + errs.addError(errMsg) + continue + } + + // Initialize the KMS code + kmsConfig := kms.NewConfig(c.context, &c.spec, c.clusterInfo) + + // We could set an env var in the Operator or a global var instead of the API call? + // Hopefully, the API is cheap and we can always retrieve the token if it has changed... + if c.spec.Security.KeyManagementService.IsTokenAuthEnabled() { + err := kms.SetTokenToEnvVar(c.context, c.spec.Security.KeyManagementService.TokenSecretName, kmsConfig.Provider, c.clusterInfo.Namespace) + if err != nil { + errMsg := fmt.Sprintf("failed to fetch kms token secret %q. %v", c.spec.Security.KeyManagementService.TokenSecretName, err) + errs.addError(errMsg) + continue + } + } + + // Generate and store the encrypted key in whatever KMS is configured + err = kmsConfig.PutSecret(osdProps.pvc.ClaimName, key) + if err != nil { + errMsg := fmt.Sprintf("failed to store secret. %v", err) + errs.addError(errMsg) + continue + } + } + + // Skip OSD prepare if deployment already exists for the PVC + if existingDeployments.Contains(dataSource.ClaimName) { + logger.Debugf("skipping OSD prepare job creation for PVC %q because OSD daemon using the PVC already exists", osdProps.crushHostname) + continue + } + + // Update the orchestration status of this pvc to the starting state + status := OrchestrationStatus{Status: OrchestrationStatusStarting, PvcBackedOSD: true} + cmName := c.updateOSDStatus(osdProps.crushHostname, status) + + if err := c.runPrepareJob(&osdProps, config); err != nil { + c.handleOrchestrationFailure(errs, osdProps.crushHostname, "%v", err) + c.deleteStatusConfigMap(osdProps.crushHostname) + continue // do not record the status CM's name + } + + // record the name of the status configmap that will eventually receive results from the + // OSD provisioning job we just created. This will help us determine when we are done + // processing the results of provisioning jobs. + awaitingStatusConfigMaps.Add(cmName) + } + + return awaitingStatusConfigMaps, nil +} + +// Returns a set of all the awaitingStatusConfigMaps that will be updated by provisioning jobs. +// Returns error only if the calling function should halt all OSD provisioning. Non-halting errors +// are added to provisionErrors. +// +// Creation of prepare jobs is most directly related to creating new OSDs. And we want to keep all +// usage of awaitingStatusConfigMaps in this file. +func (c *Cluster) startProvisioningOverNodes(config *provisionConfig, errs *provisionErrors) (*util.Set, error) { + if !c.spec.Storage.UseAllNodes && len(c.spec.Storage.Nodes) == 0 { + logger.Info("no nodes are defined for configuring OSDs on raw devices") + return util.NewSet(), nil + } + + if c.spec.Storage.UseAllNodes { + if len(c.spec.Storage.Nodes) > 0 { + logger.Warningf("useAllNodes is TRUE, but nodes are specified. NODES in the cluster CR will be IGNORED unless useAllNodes is FALSE.") + } + + // Get the list of all nodes in the cluster. The placement settings will be applied below. + hostnameMap, err := k8sutil.GetNodeHostNames(c.context.Clientset) + if err != nil { + errs.addError("failed to provision OSDs on nodes. failed to get node hostnames. %v", err) + return util.NewSet(), nil + } + c.spec.Storage.Nodes = nil + for _, hostname := range hostnameMap { + storageNode := rookv1.Node{ + Name: hostname, + } + c.spec.Storage.Nodes = append(c.spec.Storage.Nodes, storageNode) + } + logger.Debugf("storage nodes: %+v", c.spec.Storage.Nodes) + } + // generally speaking, this finds nodes which are capable of running new osds + validNodes := k8sutil.GetValidNodes(c.spec.Storage, c.context.Clientset, cephv1.GetOSDPlacement(c.spec.Placement)) + + logger.Infof("%d of the %d storage nodes are valid", len(validNodes), len(c.spec.Storage.Nodes)) + + c.ValidStorage = *c.spec.Storage.DeepCopy() + c.ValidStorage.Nodes = validNodes + + // no valid node is ready to run an osd + if len(validNodes) == 0 { + logger.Warningf("no valid nodes available to run osds on nodes in namespace %q", c.clusterInfo.Namespace) + return util.NewSet(), nil + } + + if len(c.spec.DataDirHostPath) == 0 { + errs.addError("failed to provision OSDs on nodes. user has specified valid nodes for storage, but dataDirHostPath is empty. user must set CephCluster dataDirHostPath") + return util.NewSet(), nil + } + + awaitingStatusConfigMaps := util.NewSet() + for _, node := range c.ValidStorage.Nodes { + // Check whether we need to cancel the orchestration + if err := controller.CheckForCancelledOrchestration(c.context); err != nil { + return awaitingStatusConfigMaps, err + } + + // fully resolve the storage config and resources for this node + n := c.resolveNode(node.Name) + if n == nil { + logger.Warningf("node %q did not resolve", node.Name) + continue + } + + if n.Name == "" { + logger.Warningf("skipping node with a blank name! %+v", n) + continue + } + + // create the job that prepares osds on the node + storeConfig := osdconfig.ToStoreConfig(n.Config) + metadataDevice := osdconfig.MetadataDevice(n.Config) + osdProps := osdProperties{ + crushHostname: n.Name, + devices: n.Devices, + selection: n.Selection, + resources: n.Resources, + storeConfig: storeConfig, + metadataDevice: metadataDevice, + } + + // update the orchestration status of this node to the starting state + status := OrchestrationStatus{Status: OrchestrationStatusStarting} + cmName := c.updateOSDStatus(n.Name, status) + + if err := c.runPrepareJob(&osdProps, config); err != nil { + c.handleOrchestrationFailure(errs, n.Name, "%v", err) + c.deleteStatusConfigMap(n.Name) + continue // do not record the status CM's name + } + + // record the name of the status configmap that will eventually receive results from the + // OSD provisioning job we just created. This will help us determine when we are done + // processing the results of provisioning jobs. + awaitingStatusConfigMaps.Add(cmName) + } + + return awaitingStatusConfigMaps, nil +} + +func (c *Cluster) runPrepareJob(osdProps *osdProperties, config *provisionConfig) error { + nodeOrPVC := "node" + if osdProps.onPVC() { + nodeOrPVC = "PVC" + } + nodeOrPVCName := osdProps.crushHostname + + job, err := c.makeJob(*osdProps, config) + if err != nil { + return errors.Wrapf(err, "failed to generate osd provisioning job template for %s %q", nodeOrPVC, nodeOrPVCName) + } + + if err := k8sutil.RunReplaceableJob(c.context.Clientset, job, false); err != nil { + if !kerrors.IsAlreadyExists(err) { + return errors.Wrapf(err, "failed to run provisioning job for %s %q", nodeOrPVC, nodeOrPVCName) + } + logger.Infof("letting preexisting OSD provisioning job run to completion for %s %q", nodeOrPVC, nodeOrPVCName) + return nil + } + + logger.Infof("started OSD provisioning job for %s %q", nodeOrPVC, nodeOrPVCName) + return nil +} + +func createDaemonOnPVC(c *Cluster, osd OSDInfo, pvcName string, config *provisionConfig) error { + d, err := deploymentOnPVC(c, osd, pvcName, config) + if err != nil { + return err + } + + message := fmt.Sprintf("Processing OSD %d on PVC %q", osd.ID, pvcName) + updateConditionFunc(c.context, c.clusterInfo.NamespacedName(), cephv1.ConditionProgressing, v1.ConditionTrue, cephv1.ClusterProgressingReason, message) + + _, err = k8sutil.CreateDeployment(c.context.Clientset, d) + return errors.Wrapf(err, "failed to create deployment for OSD %d on PVC %q", osd.ID, pvcName) +} + +func createDaemonOnNode(c *Cluster, osd OSDInfo, nodeName string, config *provisionConfig) error { + d, err := deploymentOnNode(c, osd, nodeName, config) + if err != nil { + return err + } + + message := fmt.Sprintf("Processing OSD %d on node %q", osd.ID, nodeName) + updateConditionFunc(c.context, c.clusterInfo.NamespacedName(), cephv1.ConditionProgressing, v1.ConditionTrue, cephv1.ClusterProgressingReason, message) + + _, err = k8sutil.CreateDeployment(c.context.Clientset, d) + return errors.Wrapf(err, "failed to create deployment for OSD %d on node %q", osd.ID, nodeName) +} diff --git a/pkg/operator/ceph/cluster/osd/create_test.go b/pkg/operator/ceph/cluster/osd/create_test.go new file mode 100644 index 000000000..20b4174af --- /dev/null +++ b/pkg/operator/ceph/cluster/osd/create_test.go @@ -0,0 +1,705 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package osd + +import ( + "context" + "strings" + "testing" + + "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + rookv1 "github.com/rook/rook/pkg/apis/rook.io/v1" + "github.com/rook/rook/pkg/clusterd" + cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + cephver "github.com/rook/rook/pkg/operator/ceph/version" + "github.com/rook/rook/pkg/operator/test" + "github.com/rook/rook/pkg/util" + "github.com/stretchr/testify/assert" + "github.com/tevino/abool" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + apiresource "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" +) + +func Test_createNewOSDsFromStatus(t *testing.T) { + namespace := "my-namespace" + + clientset := fake.NewSimpleClientset() + // clusterd.Context is created in doSetup() + + // names of status configmaps for nodes + statusNameNode0 := statusConfigMapName("node0") + statusNameNode2 := statusConfigMapName("node2") + statusNamePVC1 := statusConfigMapName("pvc1") + statusNamePVC2 := statusConfigMapName("pvc2") + + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + + var oldCreateDaemonOnNodeFunc = createDaemonOnNodeFunc + defer func() { + createDaemonOnNodeFunc = oldCreateDaemonOnNodeFunc + }() + createCallsOnNode := []int{} + induceFailureCreatingOSD := -1 // allow causing the create call to fail for a given OSD ID + createDaemonOnNodeFunc = func(c *Cluster, osd OSDInfo, nodeName string, config *provisionConfig) error { + createCallsOnNode = append(createCallsOnNode, osd.ID) + if induceFailureCreatingOSD == osd.ID { + return errors.Errorf("createOSDDaemonOnNode: induced failure on OSD %d", osd.ID) + } + return nil + } + + var oldCreateDaemonOnPVCFunc = createDaemonOnPVCFunc + defer func() { + createDaemonOnPVCFunc = oldCreateDaemonOnPVCFunc + }() + createCallsOnPVC := []int{} + // reuse induceFailureCreatingOSD from above + createDaemonOnPVCFunc = func(c *Cluster, osd OSDInfo, pvcName string, config *provisionConfig) error { + createCallsOnPVC = append(createCallsOnPVC, osd.ID) + if induceFailureCreatingOSD == osd.ID { + return errors.Errorf("createOSDDaemonOnNode: induced failure on OSD %d", osd.ID) + } + return nil + } + + // Simulate an environment where deployments exist for OSDs 3, 4, and 6 + deployments := newExistenceListWithCapacity(5) + deployments.Add(3) + deployments.Add(4) + deployments.Add(6) + + spec := cephv1.ClusterSpec{} + var status *OrchestrationStatus + awaitingStatusConfigMaps := util.NewSet() + + var c *Cluster + var createConfig *createConfig + var errs *provisionErrors + doSetup := func() { + // none of this code should ever add or remove deployments from the existence list + assert.Equal(t, 3, deployments.Len()) + // Simulate environment where provision jobs were created for node0, node2, pvc1, and pvc2 + awaitingStatusConfigMaps = util.NewSet() + awaitingStatusConfigMaps.AddMultiple([]string{ + statusNameNode0, statusNameNode2, + statusNamePVC1, statusNamePVC2}) + createCallsOnNode = createCallsOnNode[:0] + createCallsOnPVC = createCallsOnPVC[:0] + errs = newProvisionErrors() + ctx := &clusterd.Context{ + Clientset: clientset, + } + c = New(ctx, clusterInfo, spec, "rook/rook:master") + config := c.newProvisionConfig() + createConfig = c.newCreateConfig(config, awaitingStatusConfigMaps, deployments) + } + + t.Run("node: create no OSDs when none are returned from node", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{}, + PvcBackedOSD: false, + } + createConfig.createNewOSDsFromStatus(status, "node0", errs) + assert.Zero(t, errs.len()) + assert.Len(t, createCallsOnNode, 0) + assert.Len(t, createCallsOnPVC, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNameNode0)) + }) + + t.Run("test: node: create all OSDs on node when all do not exist", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 0}, {ID: 1}, {ID: 2}, + }, + PvcBackedOSD: false, + } + createConfig.createNewOSDsFromStatus(status, "node2", errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, createCallsOnNode, []int{0, 1, 2}) + assert.Len(t, createCallsOnPVC, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNameNode2)) + }) + + t.Run("node: create only nonexistent OSDs on node when some already exist", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 3}, {ID: 4}, // already exist + {ID: 5}, // does not exist + {ID: 6}, // already exists + {ID: 7}, // does not exist + }, + PvcBackedOSD: false, + } + createConfig.createNewOSDsFromStatus(status, "node0", errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, createCallsOnNode, []int{5, 7}) + assert.Len(t, createCallsOnPVC, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNameNode0)) + }) + + t.Run("node: skip creating OSDs for status configmaps that weren't created for this reconcile", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 0}, {ID: 1}, {ID: 2}, + }, + PvcBackedOSD: false, + } + createConfig.createNewOSDsFromStatus(status, "node1", errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, createCallsOnNode, []int{}) + assert.Len(t, createCallsOnPVC, 0) + // status map should NOT have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 0, createConfig.finishedStatusConfigMaps.Count()) + }) + + t.Run("node: errors reported if OSDs fail to create", func(t *testing.T) { + induceFailureCreatingOSD = 1 // fail when creating OSD 1 + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 0}, {ID: 1}, {ID: 2}, + }, + PvcBackedOSD: false, + } + createConfig.createNewOSDsFromStatus(status, "node0", errs) + assert.Equal(t, 1, errs.len()) + assert.ElementsMatch(t, createCallsOnNode, []int{0, 1, 2}) + assert.Len(t, createCallsOnPVC, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNameNode0)) + induceFailureCreatingOSD = -1 // off + }) + + t.Run("pvc: create no OSDs when none are returned from PVC", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{}, + PvcBackedOSD: true, + } + createConfig.createNewOSDsFromStatus(status, "pvc1", errs) + assert.Zero(t, errs.len()) + assert.Len(t, createCallsOnNode, 0) + assert.Len(t, createCallsOnPVC, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNamePVC1)) + }) + + t.Run("pvc: create all OSDs on pvc when all do not exist", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 0}, {ID: 1}, {ID: 2}, + }, + PvcBackedOSD: true, + } + createConfig.createNewOSDsFromStatus(status, "pvc2", errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, createCallsOnPVC, []int{0, 1, 2}) + assert.Len(t, createCallsOnNode, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNamePVC2)) + }) + + t.Run("pvc: create only nonexistent OSDs on pvc when some already exist", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 3}, {ID: 4}, // already exist + {ID: 5}, // does not exist + {ID: 6}, // already exists + {ID: 7}, // does not exist + }, + PvcBackedOSD: true, + } + createConfig.createNewOSDsFromStatus(status, "pvc1", errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, createCallsOnPVC, []int{5, 7}) + assert.Len(t, createCallsOnNode, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNamePVC1)) + }) + + t.Run("pvc: skip creating OSDs for status configmaps that weren't created for this reconcile", func(t *testing.T) { + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 0}, {ID: 1}, {ID: 2}, + }, + PvcBackedOSD: true, + } + createConfig.createNewOSDsFromStatus(status, "pvc0", errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, createCallsOnPVC, []int{}) + assert.Len(t, createCallsOnNode, 0) + // no status maps should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 0, createConfig.finishedStatusConfigMaps.Count()) + }) + + t.Run("pvc: errors reported if OSDs fail to create", func(t *testing.T) { + induceFailureCreatingOSD = 1 // fail when creating OSD 1 + doSetup() + status = &OrchestrationStatus{ + OSDs: []OSDInfo{ + {ID: 0}, {ID: 1}, {ID: 2}, + }, + PvcBackedOSD: true, + } + createConfig.createNewOSDsFromStatus(status, "pvc1", errs) + assert.Equal(t, 1, errs.len()) + assert.ElementsMatch(t, createCallsOnPVC, []int{0, 1, 2}) + assert.Len(t, createCallsOnNode, 0) + // status map should have been marked completed + assert.Equal(t, 4, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, createConfig.finishedStatusConfigMaps.Count()) + assert.True(t, createConfig.finishedStatusConfigMaps.Contains(statusNamePVC1)) + induceFailureCreatingOSD = -1 // off + }) +} + +func Test_startProvisioningOverPVCs(t *testing.T) { + namespace := "rook-ceph" + + clientset := test.NewComplexClientset(t) // fake clientset with generate name functionality + + requestCancelOrchestration := *abool.New() + // clusterd.Context is created in doSetup() + + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + CephVersion: cephver.Nautilus, + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + + spec := cephv1.ClusterSpec{} + fakeK8sVersion := "v1.13.0" + + var errs *provisionErrors + var c *Cluster + var config *provisionConfig + var awaitingStatusConfigMaps *util.Set + var err error + doSetup := func() { + test.SetFakeKubernetesVersion(clientset, fakeK8sVersion) // PVCs require k8s version v1.13+ + errs = newProvisionErrors() + ctx := &clusterd.Context{ + Clientset: clientset, + RequestCancelOrchestration: &requestCancelOrchestration, + } + c = New(ctx, clusterInfo, spec, "rook/rook:master") + config = c.newProvisionConfig() + } + + t.Run("do nothing if no storage spec is given", func(t *testing.T) { + spec = cephv1.ClusterSpec{} + doSetup() + awaitingStatusConfigMaps, err = c.startProvisioningOverPVCs(config, errs) + assert.NoError(t, err) + assert.Zero(t, awaitingStatusConfigMaps.Count()) + assert.Zero(t, errs.len()) + // no result configmaps should have been created + cms, err := clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 0) + }) + + t.Run("do nothing if device set count is zero", func(t *testing.T) { + spec = cephv1.ClusterSpec{ + Storage: rookv1.StorageScopeSpec{ + StorageClassDeviceSets: []rookv1.StorageClassDeviceSet{ + { + Name: "set1", + Count: 0, + VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ + newDummyPVC("data", namespace, "10Gi", "gp2"), + }, + }, + }, + }, + } + doSetup() + awaitingStatusConfigMaps, err = c.startProvisioningOverPVCs(config, errs) + assert.NoError(t, err) + assert.Zero(t, awaitingStatusConfigMaps.Count()) + assert.Zero(t, errs.len()) // this was not a problem with a single job but with ALL jobs + // no result configmaps should have been created + cms, err := clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 0) + }) + + t.Run("one device set with 2 PVCs", func(t *testing.T) { + spec = cephv1.ClusterSpec{ + Storage: rookv1.StorageScopeSpec{ + StorageClassDeviceSets: []rookv1.StorageClassDeviceSet{ + { + Name: "set1", + Count: 2, + VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ + newDummyPVC("data", namespace, "10Gi", "gp2"), + }, + }, + }, + }, + } + doSetup() + awaitingStatusConfigMaps, err = c.startProvisioningOverPVCs(config, errs) + assert.NoError(t, err) + assert.Equal(t, 2, awaitingStatusConfigMaps.Count()) + assert.Zero(t, errs.len()) + cms, err := clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 2) + }) + + t.Run("repeat same device set with 2 PVCs (before provisioning jobs are done and before OSD deployments are created)", func(t *testing.T) { + // spec = + doSetup() + awaitingStatusConfigMaps, err = c.startProvisioningOverPVCs(config, errs) + assert.NoError(t, err) + assert.Equal(t, 2, awaitingStatusConfigMaps.Count()) + assert.Zero(t, errs.len()) + cms, err := clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 2) // still just 2 configmaps should exist (the same 2 from before) + }) + + t.Run("error if k8s version not high enough", func(t *testing.T) { + // spec = + clientset = test.NewComplexClientset(t) // reset to empty fake k8s environment + fakeK8sVersion = "v1.12.7" + doSetup() + awaitingStatusConfigMaps, err = c.startProvisioningOverPVCs(config, errs) + assert.NoError(t, err) + assert.Equal(t, 0, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, errs.len()) + cms, err := clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 0) + fakeK8sVersion = "v1.13.0" + }) + + t.Run("request cancel orchestration", func(t *testing.T) { + // spec = + clientset = test.NewComplexClientset(t) // reset to empty fake k8s environment + requestCancelOrchestration.Set() + doSetup() + awaitingStatusConfigMaps, err = c.startProvisioningOverPVCs(config, errs) + assert.Error(t, err) + assert.Zero(t, errs.len()) + assert.Zero(t, awaitingStatusConfigMaps.Count()) + requestCancelOrchestration.UnSet() + }) + + t.Run("error if no volume claim template", func(t *testing.T) { + spec = cephv1.ClusterSpec{ + Storage: rookv1.StorageScopeSpec{ + StorageClassDeviceSets: []rookv1.StorageClassDeviceSet{ + { + Name: "set1", + Count: 2, + VolumeClaimTemplates: []corev1.PersistentVolumeClaim{}, + }, + }, + }, + } + clientset = test.NewComplexClientset(t) // reset to empty fake k8s environment + doSetup() + awaitingStatusConfigMaps, err = c.startProvisioningOverPVCs(config, errs) + assert.NoError(t, err) + assert.Equal(t, 0, awaitingStatusConfigMaps.Count()) + assert.Equal(t, 1, errs.len()) + cms, err := clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 0) + }) + + // TODO: should we verify the osdProps set on the job? +} + +func Test_startProvisioningOverNodes(t *testing.T) { + namespace := "rook-ceph" + dataDirHostPath := "/var/lib/mycluster" + + clientset := test.New(t, 3) // fake clientset with 3 nodes + requestCancelOrchestration := *abool.New() + // clusterd.Context is created in doSetup() + + // names of status configmaps for nodes + statusNameNode0 := statusConfigMapName("node0") + statusNameNode1 := statusConfigMapName("node1") + statusNameNode2 := statusConfigMapName("node2") + // statusNameNode3 := statusConfigMapName("node3") + + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + CephVersion: cephver.Nautilus, + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + + var useAllDevices bool + spec := cephv1.ClusterSpec{} + + var errs *provisionErrors + var c *Cluster + var config *provisionConfig + var prepareJobsRun *util.Set + var err error + var cms *corev1.ConfigMapList + doSetup := func() { + errs = newProvisionErrors() + ctx := &clusterd.Context{ + Clientset: clientset, + RequestCancelOrchestration: &requestCancelOrchestration, + } + c = New(ctx, clusterInfo, spec, "rook/rook:master") + config = c.newProvisionConfig() + } + + t.Run("do nothing if no storage spec is given", func(t *testing.T) { + spec = cephv1.ClusterSpec{} + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, prepareJobsRun.Count()) + assert.Zero(t, errs.len()) + // no result configmaps should have been created + cms, err = clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 0) + }) + + t.Run("error on empty dataDirHostPath", func(t *testing.T) { + useAllDevices = true + spec = cephv1.ClusterSpec{ + // this storage spec should cause prepare jobs to run on all (3) nodes + Storage: rookv1.StorageScopeSpec{ + UseAllNodes: true, + Selection: rookv1.Selection{ + UseAllDevices: &useAllDevices, + }, + }, + // BUT empty should not allow any jobs to be created + DataDirHostPath: "", + } + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, prepareJobsRun.Count()) + assert.Equal(t, 1, errs.len()) // this was not a problem with a single job but with ALL jobs + // no result configmaps should have been created + cms, err = clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 0) + }) + + t.Run("use all nodes and devices", func(t *testing.T) { + // Setting dataDirHostPath non-empty on the previous config should have jobs run for all nodes + spec.DataDirHostPath = dataDirHostPath + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, + []string{statusNameNode0, statusNameNode1, statusNameNode2}, + prepareJobsRun.ToSlice(), + ) + // all result configmaps should have been created + cms, err = clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 3) + }) + + t.Run("use all nodes and devices when useAllNodes and individual nodes are both set", func(t *testing.T) { + // this also tests that jobs that currently exist (created in previous test) are handled + spec.Storage.Nodes = []rookv1.Node{ + {Name: "node0"}, {Name: "node2"}, + } + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, + []string{statusNameNode0, statusNameNode1, statusNameNode2}, + prepareJobsRun.ToSlice(), + ) + }) + + t.Run("use individual nodes", func(t *testing.T) { + // this also tests that existing status configmaps (from the previous tests) don't affect the + // reported status configmaps from this run + spec = cephv1.ClusterSpec{ + Storage: rookv1.StorageScopeSpec{ + UseAllNodes: false, + Nodes: []rookv1.Node{ + // run on only node0 and node2 + {Name: "node0"}, + {Name: "node2"}, + }, + Selection: rookv1.Selection{ + UseAllDevices: &useAllDevices, + }, + }, + DataDirHostPath: dataDirHostPath, + } + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, + []string{statusNameNode0, statusNameNode2}, + prepareJobsRun.ToSlice(), + ) + }) + + t.Run("request cancel orchestration", func(t *testing.T) { + requestCancelOrchestration.Set() + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.Error(t, err) + assert.Zero(t, errs.len()) + assert.Zero(t, prepareJobsRun.Count()) + requestCancelOrchestration.UnSet() + }) + + t.Run("use no nodes", func(t *testing.T) { + spec = cephv1.ClusterSpec{ + Storage: rookv1.StorageScopeSpec{ + UseAllNodes: false, + Nodes: []rookv1.Node{ + // empty + }, + Selection: rookv1.Selection{ + UseAllDevices: &useAllDevices, + }, + }, + DataDirHostPath: dataDirHostPath, + } + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.Zero(t, prepareJobsRun.Count()) + }) + + t.Run("failures running prepare jobs", func(t *testing.T) { + spec = cephv1.ClusterSpec{ + Storage: rookv1.StorageScopeSpec{ + UseAllNodes: false, + Nodes: []rookv1.Node{ + // run on only node0 and node2 + {Name: "node0"}, + {Name: "node2"}, + }, + Selection: rookv1.Selection{ + UseAllDevices: &useAllDevices, + }, + }, + DataDirHostPath: dataDirHostPath, + } + // re-initialize an empty test clientset with 3 nodes + clientset = test.New(t, 3) + // add a job reactor that will cause the node2 job to fail + var jobReactor k8stesting.ReactionFunc = func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + switch action := action.(type) { + case k8stesting.CreateActionImpl: + obj := action.GetObject() + objMeta, err := meta.Accessor(obj) + if err != nil { + panic(err) + } + objName := objMeta.GetName() + if strings.Contains(objName, "node2") { + return true, nil, errors.Errorf("induced error") + } + default: + panic("this should not happen") + } + return false, nil, nil + } + clientset.PrependReactor("create", "jobs", jobReactor) + doSetup() + prepareJobsRun, err = c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Equal(t, 1, errs.len()) + assert.ElementsMatch(t, + []string{statusNameNode0}, + prepareJobsRun.ToSlice(), + ) + // with a fresh clientset, only the one results ConfigMap should exist + cms, err = clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cms.Items, 1) + assert.Equal(t, prepareJobsRun.ToSlice()[0], cms.Items[0].Name) + }) +} + +func newDummyPVC(name, namespace string, capacity string, storageClassName string) corev1.PersistentVolumeClaim { + volMode := corev1.PersistentVolumeBlock + return corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceName(corev1.ResourceStorage): apiresource.MustParse(capacity), + }, + }, + StorageClassName: &storageClassName, + VolumeMode: &volMode, + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + }, + } +} diff --git a/pkg/operator/ceph/cluster/osd/deviceSet.go b/pkg/operator/ceph/cluster/osd/deviceSet.go index 7e867ac29..7589eac6b 100644 --- a/pkg/operator/ceph/cluster/osd/deviceSet.go +++ b/pkg/operator/ceph/cluster/osd/deviceSet.go @@ -32,24 +32,24 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func (c *Cluster) prepareStorageClassDeviceSets(config *provisionConfig) []rookv1.VolumeSource { +func (c *Cluster) prepareStorageClassDeviceSets(errs *provisionErrors) []rookv1.VolumeSource { volumeSources := []rookv1.VolumeSource{} existingPVCs, uniqueOSDsPerDeviceSet, err := GetExistingPVCs(c.context, c.clusterInfo.Namespace) if err != nil { - config.addError("failed to detect existing OSD PVCs. %v", err) + errs.addError("failed to detect existing OSD PVCs. %v", err) return volumeSources } // Iterate over deviceSet for _, deviceSet := range c.spec.Storage.StorageClassDeviceSets { if err := controller.CheckPodMemory(cephv1.ResourcesKeyPrepareOSD, deviceSet.Resources, cephOsdPodMinimumMemory); err != nil { - config.addError("cannot use device set %q for creating osds %v", deviceSet.Name, err) + errs.addError("failed to provision OSDs on PVC for storageClassDeviceSet %q. %v", deviceSet.Name, err) continue } // Check if the volume claim template is specified if len(deviceSet.VolumeClaimTemplates) == 0 { - logger.Warningf("no volumeClaimTemplate specified for device set %q", deviceSet.Name) + errs.addError("failed to provision OSDs on PVC for storageClassDeviceSet %q. no volumeClaimTemplate is specified. user must specify a volumeClaimTemplate", deviceSet.Name) continue } @@ -57,18 +57,18 @@ func (c *Cluster) prepareStorageClassDeviceSets(config *provisionConfig) []rookv highestExistingID := -1 countInDeviceSet := 0 if existingIDs, ok := uniqueOSDsPerDeviceSet[deviceSet.Name]; ok { - logger.Infof("verifying pvcs exist for %d osds in device set %q", existingIDs.Count(), deviceSet.Name) + logger.Infof("verifying PVCs exist for %d OSDs in device set %q", existingIDs.Count(), deviceSet.Name) for existingID := range existingIDs.Iter() { pvcID, err := strconv.Atoi(existingID) if err != nil { - config.addError("invalid PVC index %q found for device set %q", existingID, deviceSet.Name) + errs.addError("invalid PVC index %q found for device set %q", existingID, deviceSet.Name) continue } // keep track of the max PVC index found so we know what index to start with for new OSDs if pvcID > highestExistingID { highestExistingID = pvcID } - volumeSource := c.createDeviceSetPVCsForIndex(config, deviceSet, existingPVCs, pvcID) + volumeSource := c.createDeviceSetPVCsForIndex(deviceSet, existingPVCs, pvcID, errs) volumeSources = append(volumeSources, volumeSource) } countInDeviceSet = existingIDs.Count() @@ -81,7 +81,7 @@ func (c *Cluster) prepareStorageClassDeviceSets(config *provisionConfig) []rookv } for i := 0; i < pvcsToCreate; i++ { pvcID := highestExistingID + i + 1 - volumeSource := c.createDeviceSetPVCsForIndex(config, deviceSet, existingPVCs, pvcID) + volumeSource := c.createDeviceSetPVCsForIndex(deviceSet, existingPVCs, pvcID, errs) volumeSources = append(volumeSources, volumeSource) countInDeviceSet++ } @@ -90,7 +90,7 @@ func (c *Cluster) prepareStorageClassDeviceSets(config *provisionConfig) []rookv return volumeSources } -func (c *Cluster) createDeviceSetPVCsForIndex(config *provisionConfig, deviceSet rookv1.StorageClassDeviceSet, existingPVCs map[string]*v1.PersistentVolumeClaim, setIndex int) rookv1.VolumeSource { +func (c *Cluster) createDeviceSetPVCsForIndex(deviceSet rookv1.StorageClassDeviceSet, existingPVCs map[string]*v1.PersistentVolumeClaim, setIndex int, errs *provisionErrors) rookv1.VolumeSource { // Create the PVC source for each of the data, metadata, and other types of templates if defined. pvcSources := map[string]v1.PersistentVolumeClaimVolumeSource{} @@ -104,7 +104,7 @@ func (c *Cluster) createDeviceSetPVCsForIndex(config *provisionConfig, deviceSet pvc, err := c.createDeviceSetPVC(existingPVCs, deviceSet.Name, pvcTemplate, setIndex) if err != nil { - config.addError("failed to create osd for device set %q for count %d. %v", deviceSet.Name, setIndex, err) + errs.addError("failed to provision PVC for device set %q index %d. %v", deviceSet.Name, setIndex, err) continue } @@ -172,7 +172,7 @@ func (c *Cluster) createDeviceSetPVC(existingPVCs map[string]*v1.PersistentVolum // No PVC found, creating a new one deployedPVC, err := c.context.Clientset.CoreV1().PersistentVolumeClaims(c.clusterInfo.Namespace).Create(ctx, pvc, metav1.CreateOptions{}) if err != nil { - return nil, errors.Wrapf(err, "failed to create pvc %q for device set %q", pvc.Name, deviceSetName) + return nil, errors.Wrapf(err, "failed to create PVC %q for device set %q", pvc.Name, deviceSetName) } logger.Infof("successfully provisioned PVC %q", deployedPVC.Name) @@ -184,20 +184,20 @@ func (c *Cluster) updatePVCIfChanged(desiredPVC *v1.PersistentVolumeClaim, curre desiredSize, desiredOK := desiredPVC.Spec.Resources.Requests[v1.ResourceStorage] currentSize, currentOK := currentPVC.Spec.Resources.Requests[v1.ResourceStorage] if !desiredOK || !currentOK { - logger.Debugf("desired or current size are not specified for pvc %q", currentPVC.Name) + logger.Debugf("desired or current size are not specified for PVC %q", currentPVC.Name) return } if desiredSize.Value() > currentSize.Value() { currentPVC.Spec.Resources.Requests[v1.ResourceStorage] = desiredSize - logger.Infof("updating pvc %q size from %s to %s", currentPVC.Name, currentSize.String(), desiredSize.String()) + logger.Infof("updating PVC %q size from %s to %s", currentPVC.Name, currentSize.String(), desiredSize.String()) if _, err := c.context.Clientset.CoreV1().PersistentVolumeClaims(c.clusterInfo.Namespace).Update(ctx, currentPVC, metav1.UpdateOptions{}); err != nil { // log the error, but don't fail the reconcile - logger.Errorf("failed to update pvc size. %v", err) + logger.Errorf("failed to update PVC size. %v", err) return } - logger.Infof("successfully updated pvc %q size", currentPVC.Name) + logger.Infof("successfully updated PVC %q size", currentPVC.Name) } else if desiredSize.Value() < currentSize.Value() { - logger.Warningf("ignoring request to shrink osd pvc %q size from %s to %s, only expansion is allowed", currentPVC.Name, currentSize.String(), desiredSize.String()) + logger.Warningf("ignoring request to shrink osd PVC %q size from %s to %s, only expansion is allowed", currentPVC.Name, currentSize.String(), desiredSize.String()) } } @@ -229,7 +229,7 @@ func GetExistingPVCs(clusterdContext *clusterd.Context, namespace string) (map[s selector := metav1.ListOptions{LabelSelector: CephDeviceSetPVCIDLabelKey} pvcs, err := clusterdContext.Clientset.CoreV1().PersistentVolumeClaims(namespace).List(ctx, selector) if err != nil { - return nil, nil, errors.Wrap(err, "failed to detect pvcs") + return nil, nil, errors.Wrap(err, "failed to detect PVCs") } result := map[string]*v1.PersistentVolumeClaim{} uniqueOSDsPerDeviceSet := map[string]*util.Set{} diff --git a/pkg/operator/ceph/cluster/osd/deviceset_test.go b/pkg/operator/ceph/cluster/osd/deviceset_test.go index a83c33f22..e905b609f 100644 --- a/pkg/operator/ceph/cluster/osd/deviceset_test.go +++ b/pkg/operator/ceph/cluster/osd/deviceset_test.go @@ -66,10 +66,10 @@ func testPrepareDeviceSets(t *testing.T, setTemplateName bool) { spec: spec, } - config := &provisionConfig{} - volumeSources := cluster.prepareStorageClassDeviceSets(config) + errs := newProvisionErrors() + volumeSources := cluster.prepareStorageClassDeviceSets(errs) assert.Equal(t, 1, len(volumeSources)) - assert.Equal(t, 0, len(config.errorMessages)) + assert.Equal(t, 0, errs.len()) assert.Equal(t, "mydata", volumeSources[0].Name) assert.True(t, volumeSources[0].Portable) _, dataOK := volumeSources[0].PVCSources["data"] @@ -141,10 +141,10 @@ func TestPrepareDeviceSetWithHolesInPVCs(t *testing.T) { clientset.PrependReactor("create", "persistentvolumeclaims", pvcReactor) // Create 3 PVCs for two OSDs in the device set - config := &provisionConfig{} + config := newProvisionErrors() volumeSources := cluster.prepareStorageClassDeviceSets(config) assert.Equal(t, 1, len(volumeSources)) - assert.Equal(t, 0, len(config.errorMessages)) + assert.Equal(t, 0, config.len()) assert.Equal(t, "mydata", volumeSources[0].Name) assert.True(t, volumeSources[0].Portable) _, dataOK := volumeSources[0].PVCSources["data"] @@ -162,7 +162,7 @@ func TestPrepareDeviceSetWithHolesInPVCs(t *testing.T) { cluster.spec.Storage.StorageClassDeviceSets[0].Count = 2 volumeSources = cluster.prepareStorageClassDeviceSets(config) assert.Equal(t, 2, len(volumeSources)) - assert.Equal(t, 0, len(config.errorMessages)) + assert.Equal(t, 0, config.len()) // Verify the PVCs all exist pvcs, err = clientset.CoreV1().PersistentVolumeClaims(cluster.clusterInfo.Namespace).List(ctx, metav1.ListOptions{}) diff --git a/pkg/operator/ceph/cluster/osd/envs.go b/pkg/operator/ceph/cluster/osd/envs.go index fd23d10d0..677fcea76 100644 --- a/pkg/operator/ceph/cluster/osd/envs.go +++ b/pkg/operator/ceph/cluster/osd/envs.go @@ -127,6 +127,10 @@ func metadataDeviceEnvVar(metadataDevice string) v1.EnvVar { return v1.EnvVar{Name: osdMetadataDeviceEnvVarName, Value: metadataDevice} } +func walDeviceEnvVar(walDevice string) v1.EnvVar { + return v1.EnvVar{Name: osdWalDeviceEnvVarName, Value: walDevice} +} + func pvcBackedOSDEnvVar(pvcBacked string) v1.EnvVar { return v1.EnvVar{Name: PVCBackedOSDVarName, Value: pvcBacked} } diff --git a/pkg/operator/ceph/cluster/osd/health.go b/pkg/operator/ceph/cluster/osd/health.go index d5c448e73..5ebd85662 100644 --- a/pkg/operator/ceph/cluster/osd/health.go +++ b/pkg/operator/ceph/cluster/osd/health.go @@ -80,7 +80,7 @@ func (m *OSDHealthMonitor) Start(stopCh chan struct{}) { m.checkOSDHealth() case <-stopCh: - logger.Infof("Stopping monitoring of OSDs in namespace %s", m.clusterInfo.Namespace) + logger.Infof("Stopping monitoring of OSDs in namespace %q", m.clusterInfo.Namespace) return } } diff --git a/pkg/operator/ceph/cluster/osd/integration_test.go b/pkg/operator/ceph/cluster/osd/integration_test.go new file mode 100644 index 000000000..b08c89ea6 --- /dev/null +++ b/pkg/operator/ceph/cluster/osd/integration_test.go @@ -0,0 +1,679 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package osd + +import ( + "context" + "encoding/json" + "fmt" + "os" + "strconv" + "strings" + "testing" + "time" + + "github.com/coreos/pkg/capnslog" + "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + rookv1 "github.com/rook/rook/pkg/apis/rook.io/v1" + "github.com/rook/rook/pkg/clusterd" + cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + cephclientfake "github.com/rook/rook/pkg/daemon/ceph/client/fake" + cephver "github.com/rook/rook/pkg/operator/ceph/version" + "github.com/rook/rook/pkg/operator/k8sutil" + "github.com/rook/rook/pkg/operator/test" + exectest "github.com/rook/rook/pkg/util/exec/test" + "github.com/stretchr/testify/assert" + "github.com/tevino/abool" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" +) + +var ( + // global to allow creating helper functions that are not inline with test functions + testIDGenerator osdIDGenerator +) + +// TODO: look into: failed to calculate diff between current deployment and newly generated one. +// Failed to generate strategic merge patch: map: map[] does not contain declared merge key: uid +// appears in unit tests, but does it appear in real-world? + +// Test integration between OSD creates and updates including on both nodes and PVCs. +// The definition for this test is a wrapper for the test function that adds a timeout. +func TestOSDIntegration(t *testing.T) { + oldLogger := *logger + defer func() { logger = &oldLogger }() // reset logger to default after this test + logger.SetLevel(capnslog.TRACE) // want more log info for this test if it fails + + oldOpportunisticDuration := osdOpportunisticUpdateDuration + oldMinuteDuration := minuteTickerDuration + defer func() { + osdOpportunisticUpdateDuration = oldOpportunisticDuration + minuteTickerDuration = oldMinuteDuration + }() + // lower the check durations for unit tests to speed them up + osdOpportunisticUpdateDuration = 1 * time.Millisecond + minuteTickerDuration = 3 * time.Millisecond + + done := make(chan bool) + // runs in less than 650ms on 6-core, 16MB RAM system, but github CI can be much slower + timeout := time.After(20 * 750 * time.Millisecond) + + go func() { + // use defer because t.Fatal will kill this goroutine, and we always want done set if the + // test func stops running + defer func() { done <- true }() + // run the actual test + testOSDIntegration(t) + }() + + select { + case <-timeout: + t.Fatal("Test timed out. This is a test failure.") + case <-done: + } +} + +// This is the actual test. If it hangs, we should consider that an error. +func testOSDIntegration(t *testing.T) { + ctx := context.TODO() + namespace := "osd-integration" + clusterName := "my-cluster" + + testIDGenerator = newOSDIDGenerator() + + // mock/stub functions as needed + oldConditionExportFunc := updateConditionFunc + defer func() { + updateConditionFunc = oldConditionExportFunc + }() + // stub out the conditionExportFunc to do nothing. we do not have a fake Rook interface that + // allows us to interact with a CephCluster resource like the fake K8s clientset. + updateConditionFunc = func(c *clusterd.Context, namespaceName types.NamespacedName, conditionType cephv1.ConditionType, status corev1.ConditionStatus, reason cephv1.ClusterReasonType, message string) { + // do nothing + } + + // set up a fake k8s client set and watcher to generate events that the operator will listen to + clientset := test.NewComplexClientset(t) + test.AddSomeReadyNodes(t, clientset, 3) + assignPodToNode := true + test.PrependComplexJobReactor(t, clientset, assignPodToNode) + test.SetFakeKubernetesVersion(clientset, "v1.13.2") // v1.13 or higher is required for OSDs on PVC + + os.Setenv(k8sutil.PodNamespaceEnvVar, namespace) + defer os.Unsetenv(k8sutil.PodNamespaceEnvVar) + + statusMapWatcher := watch.NewRaceFreeFake() + clientset.PrependWatchReactor("configmaps", k8stesting.DefaultWatchReactor(statusMapWatcher, nil)) + + failCreatingDeployments := []string{} + failUpdatingDeployments := []string{} + deploymentGeneration := int64(1) // mock deployment generation constantly increasing + deploymentsCreated := []string{} + deploymentsUpdated := []string{} + var deploymentReactor k8stesting.ReactionFunc = func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + switch action := action.(type) { + case k8stesting.CreateActionImpl: + obj := action.GetObject() + d, ok := obj.(*appsv1.Deployment) + if !ok { + panic(fmt.Sprintf("object not a deployment: %+v", obj)) + } + t.Logf("deployment reactor: create event for deployment %q", d.Name) + // (1) keep track of deployments which have been created + deploymentsCreated = append(deploymentsCreated, d.Name) + // (2) set deployments ready immediately. Don't have to test waiting for deployments to + // be ready b/c that is tested thoroughly in update_test.go + d.Status.ObservedGeneration = deploymentGeneration + d.Status.UpdatedReplicas = 1 + d.Status.ReadyReplicas = 1 + deploymentGeneration++ + // (3) return a failure if asked + for _, match := range failCreatingDeployments { + if strings.Contains(d.Name, match) { + return true, nil, errors.Errorf("induced error creating deployment %q", d.Name) + } + } + + case k8stesting.UpdateActionImpl: + obj := action.GetObject() + d, ok := obj.(*appsv1.Deployment) + if !ok { + panic(fmt.Sprintf("object not a deployment: %+v", obj)) + } + t.Logf("deployment reactor: update event for deployment %q", d.Name) + // (1) keep track of deployments which have been created + deploymentsUpdated = append(deploymentsUpdated, d.Name) + // (2) set deployments ready immediately. Don't have to test waiting for deployments to + // be ready b/c that is tested thoroughly in update_test.go + d.Status.ObservedGeneration = deploymentGeneration + d.Status.UpdatedReplicas = 1 + d.Status.ReadyReplicas = 1 + deploymentGeneration++ + // (3) return a failure if asked + for _, match := range failUpdatingDeployments { + if strings.Contains(d.Name, match) { + return true, nil, errors.Errorf("induced error creating deployment %q", d.Name) + } + } + + case k8stesting.DeleteActionImpl: + panic(fmt.Sprintf("deployments should not be deleted: %+v", action)) + } + + // modify the object in-place so that the default reactor will create it with our + // modifications, if we have made any + return false, nil, nil + } + clientset.PrependReactor("*", "deployments", deploymentReactor) + + clusterInfo := cephclient.NewClusterInfo(namespace, clusterName) + clusterInfo.CephVersion = cephver.Pacific + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + executor := osdIntegrationTestExecutor(t, clientset, namespace) + + context := &clusterd.Context{ + Clientset: clientset, + ConfigDir: "/var/lib/rook", + Executor: executor, + RequestCancelOrchestration: abool.New(), + } + spec := cephv1.ClusterSpec{ + CephVersion: cephv1.CephVersionSpec{ + Image: "ceph/ceph:v16.2.0", + }, + DataDirHostPath: context.ConfigDir, + // This storage spec should... (see inline) + Storage: rookv1.StorageScopeSpec{ + UseAllNodes: true, + Selection: rookv1.Selection{ + // ... create 2 osd on each of the 3 nodes (6 total) + DeviceFilter: "vd[ab]", + }, + StorageClassDeviceSets: []rookv1.StorageClassDeviceSet{ + // ... create 6 portable osds + newDummyStorageClassDeviceSet("portable-set", namespace, 6, true, "ec2"), + // ... create 3 local, non-portable osds, one on each node + newDummyStorageClassDeviceSet("local-set", namespace, 3, false, "local"), + }, + }, + } + osdsPerNode := 2 // vda and vdb + + c := New(context, clusterInfo, spec, "myversion") + + var startErr error + var done bool + runReconcile := func() { + // reset environment + c = New(context, clusterInfo, spec, "myversion") + statusMapWatcher.Reset() + + // reset counters + deploymentsCreated = []string{} + deploymentsUpdated = []string{} + done = false + + startErr = c.Start() + done = true + } + waitForDone := func() { + for { + if done == true { + return + } + time.Sleep(1 * time.Millisecond) + } + } + + // NOTE: these tests all use the same environment + t.Run("initial creation", func(t *testing.T) { + go runReconcile() + + cms := waitForNumConfigMaps(clientset, namespace, 12) // 3 nodes + 9 new PVCs + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 15) + assert.Len(t, deploymentsUpdated, 0) + }) + + t.Run("reconcile again with no changes", func(t *testing.T) { + go runReconcile() + + cms := waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 0) + assert.Len(t, deploymentsUpdated, 15) + }) + + t.Run("increase number of OSDs", func(t *testing.T) { + spec.Storage.Selection.DeviceFilter = "/dev/vd[abc]" // 3 more (1 more per node) + spec.Storage.StorageClassDeviceSets[0].Count = 8 // 2 more portable + spec.Storage.StorageClassDeviceSets[1].Count = 6 // 3 more (1 more per node) + osdsPerNode = 3 // vda, vdb, vdc + + go runReconcile() + + cms := waitForNumConfigMaps(clientset, namespace, 8) // 3 nodes + 5 new PVCs + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 8) + assert.Len(t, deploymentsUpdated, 15) + }) + + t.Run("mixed create and update, cancel reconcile, and continue reconcile", func(t *testing.T) { + spec.Storage.Selection.DeviceFilter = "/dev/vd[abcd]" // 3 more (1 more per node) + spec.Storage.StorageClassDeviceSets[0].Count = 10 // 2 more portable + osdsPerNode = 4 // vd[a-d] + + go runReconcile() + cms := waitForNumConfigMaps(clientset, namespace, 5) // 3 nodes + 2 new PVCs + i := 1 + for _, cm := range cms { + if !strings.Contains(cm.Name, "node") { + // only do node configmaps right now since those are always created and we want + // a deterministic number of configmaps in the next step + continue + } + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + if i == 2 { + t.Log("canceling orchestration") + time.Sleep(10 * time.Millisecond) + // after the second status map is made ready, cancel the orchestration. wait a short + // while to make sure the watcher picks up the updated change + c.context.RequestCancelOrchestration.Set() + break + } + i++ + } + waitForDone() + assert.Error(t, startErr) + t.Logf("c.Start() error: %+v", startErr) + // should have created 2 more OSDs for the configmaps we updated + assert.Len(t, deploymentsCreated, 2) + // we don't know exactly how many updates might have happened by this point + numUpdates := len(deploymentsUpdated) + t.Logf("deployments updated: %d", numUpdates) + + go runReconcile() + cms = waitForNumConfigMaps(clientset, namespace, 5) // 3 nodes + 2 new PVCs + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 3) // 5 less the 2 created in the previous step + assert.Len(t, deploymentsUpdated, 25) // 23 + 2 created in previous step + }) + + t.Run("failures reported in status configmaps", func(t *testing.T) { + spec.Storage.Selection.DeviceFilter = "/dev/vd[abcde]" // 3 more (1 more per node) + osdsPerNode = 5 // vd[a-e] + + go runReconcile() + cms := waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + if strings.Contains(cm.Name, "node1") { + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + } else { + setStatusConfigMapToFailed(t, cpy) // fail on node0 and node2 + } + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.Error(t, startErr) + t.Logf("c.Start() error: %+v", startErr) + assert.Len(t, deploymentsCreated, 1) + assert.Len(t, deploymentsUpdated, 28) + + // should get back to healthy after + go runReconcile() + cms = waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 2) + assert.Len(t, deploymentsUpdated, 29) // 28 + 1 created in previous step + }) + + t.Run("failures during deployment updates", func(t *testing.T) { + failUpdatingDeployments = []string{"osd-15", "osd-22"} + go runReconcile() + cms := waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.Error(t, startErr) + t.Logf("c.Start() error: %+v", startErr) + assert.Len(t, deploymentsCreated, 0) + assert.Len(t, deploymentsUpdated, 31) // should attempt to update all deployments + + failUpdatingDeployments = []string{} + go runReconcile() + cms = waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 0) + assert.Len(t, deploymentsUpdated, 31) // all deployments should be updated again + }) + + t.Run("failures during deployment creation", func(t *testing.T) { + spec.Storage.Selection.DeviceFilter = "/dev/vd[abcdef]" // 3 more (1 more per node) + osdsPerNode = 6 // vd[a-f] + + failCreatingDeployments = []string{"osd-31", "osd-33"} + go runReconcile() + cms := waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.Error(t, startErr) + t.Logf("c.Start() error: %+v", startErr) + assert.Len(t, deploymentsCreated, 3) + assert.Len(t, deploymentsUpdated, 31) + + failCreatingDeployments = []string{} + go runReconcile() + cms = waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 2) // the 2 deployments NOT created before should be created now + assert.Len(t, deploymentsUpdated, 32) // 31 + 1 from previous step + }) + + t.Run("failures from improperly formatted StorageClassDeviceSet", func(t *testing.T) { + newSCDS := rookv1.StorageClassDeviceSet{ + Name: "new", + Count: 3, + Portable: true, + VolumeClaimTemplates: []corev1.PersistentVolumeClaim{}, + } + spec.Storage.StorageClassDeviceSets = append(spec.Storage.StorageClassDeviceSets, newSCDS) + + go runReconcile() + cms := waitForNumConfigMaps(clientset, namespace, 3) // 3 nodes + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.Error(t, startErr) + t.Logf("c.Start() error: %+v", startErr) + assert.Len(t, deploymentsCreated, 0) + assert.Len(t, deploymentsUpdated, 34) + + spec.Storage.StorageClassDeviceSets[2].VolumeClaimTemplates = []corev1.PersistentVolumeClaim{ + newDummyPVC("data", namespace, "100Gi", "ec2"), + newDummyPVC("metadata", namespace, "10Gi", "uncle-rogers-secret-stuff"), + } + + go runReconcile() + cms = waitForNumConfigMaps(clientset, namespace, 6) // 3 nodes + 3 new PVCs + for _, cm := range cms { + cpy := cm.DeepCopy() + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.NoError(t, startErr) + assert.Len(t, deploymentsCreated, 3) + assert.Len(t, deploymentsUpdated, 34) + }) + + t.Run("clean up dangling configmaps", func(t *testing.T) { + danglingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dangling-status-configmap", + Namespace: namespace, + Labels: statusConfigMapLabels("node0"), + }, + } + _, err := clientset.CoreV1().ConfigMaps(namespace).Create(ctx, danglingCM, metav1.CreateOptions{}) + assert.NoError(t, err) + + go runReconcile() + cms := waitForNumConfigMaps(clientset, namespace, 4) // 3 nodes + dangling + for _, cm := range cms { + cpy := cm.DeepCopy() + if cpy.Name == "dangling-status-configmap" { + continue + } + setStatusConfigMapToCompleted(t, cpy, osdsPerNode) + updateStatusConfigmap(clientset, statusMapWatcher, cpy) + } + waitForDone() + assert.NoError(t, err) + assert.Len(t, deploymentsCreated, 0) + assert.Len(t, deploymentsUpdated, 37) + + cmList, err := clientset.CoreV1().ConfigMaps(namespace).List(ctx, metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, cmList.Items, 0) + }) +} + +/* + * mock executor to handle ceph commands + */ + +func osdIntegrationTestExecutor(t *testing.T, clientset *fake.Clientset, namespace string) *exectest.MockExecutor { + return &exectest.MockExecutor{ + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { + t.Logf("command: %s %v", command, args) + if command != "ceph" { + return "", errors.Errorf("unexpected command %q with args %v", command, args) + } + if args[0] == "auth" { + if args[1] == "get-or-create-key" { + return "{\"key\": \"does-not-matter\"}", nil + } + } + if args[0] == "osd" { + if args[1] == "ok-to-stop" { + osdID := args[2] + id, err := strconv.Atoi(osdID) + if err != nil { + panic(err) + } + t.Logf("returning ok for OSD %d", id) + return cephclientfake.OsdOkToStopOutput(id, []int{id}, true), nil + } + if args[1] == "ls" { + // ceph osd ls returns an array of osd IDs like [0,1,2] + // build this based on the number of deployments since they should be equal + // for this test + l, err := clientset.AppsV1().Deployments(namespace).List(context.TODO(), metav1.ListOptions{}) + if err != nil { + panic(fmt.Sprintf("failed to build 'ceph osd ls' output. %v", err)) + } + return cephclientfake.OsdLsOutput(len(l.Items)), nil + } + if args[1] == "tree" { + return cephclientfake.OsdTreeOutput(3, 3), nil // fake output for cluster with 3 nodes having 3 OSDs + } + } + if args[0] == "versions" { + // the update deploy code only cares about the mons from the ceph version command results + v := `{"mon":{"ceph version 16.2.2 (somehash) nautilus (stable)":3}}` + return v, nil + } + return "", errors.Errorf("unexpected ceph command %q", args) + }, + } +} + +/* + * Unique and consistent OSD ID generator + */ + +type osdIDGenerator struct { + nextOSDID int + osdIDMap map[string]int +} + +func newOSDIDGenerator() osdIDGenerator { + return osdIDGenerator{ + nextOSDID: 0, + osdIDMap: map[string]int{}, + } +} + +func (g *osdIDGenerator) osdID(t *testing.T, namedResource string) int { + if id, ok := g.osdIDMap[namedResource]; ok { + t.Logf("resource %q has existing OSD ID %d", namedResource, id) + return id + } + id := g.nextOSDID + g.osdIDMap[namedResource] = id + g.nextOSDID++ + t.Logf("generated new OSD ID %d for resource %q", id, namedResource) + return id +} + +func newDummyStorageClassDeviceSet( + name string, namespace string, count int, portable bool, storageClassName string, +) rookv1.StorageClassDeviceSet { + return rookv1.StorageClassDeviceSet{ + Name: name, + Count: count, + Portable: portable, + VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ + newDummyPVC("data", namespace, "10Gi", storageClassName), + }, + } +} + +func waitForNumConfigMaps(clientset kubernetes.Interface, namespace string, count int) []corev1.ConfigMap { + for { + cms, err := clientset.CoreV1().ConfigMaps(namespace).List(context.TODO(), metav1.ListOptions{}) + if err != nil { + panic(err) + } + if len(cms.Items) >= count { + return cms.Items + } + time.Sleep(1 * time.Microsecond) + } +} + +// Helper method to set "completed" status on "starting" ConfigMaps. +// If the configmap is a result for a provision on nodes, create numOSDsIfOnNode OSDs in the return +func setStatusConfigMapToCompleted(t *testing.T, cm *corev1.ConfigMap, numOSDsIfOnNode int) { + status := parseOrchestrationStatus(cm.Data) + t.Logf("updating configmap %q status to completed", cm.Name) + // configmap names are deterministic can be mapped indirectly to an OSD ID, and since the + // configmaps are used to report completion status of OSD provisioning, we use this property in + // these unit tests + status.Status = OrchestrationStatusCompleted + if status.PvcBackedOSD { + // only one OSD per PVC + osdID := testIDGenerator.osdID(t, cm.Name) + status.OSDs = []OSDInfo{ + { + ID: osdID, + UUID: fmt.Sprintf("%032d", osdID), + BlockPath: "/dev/path/to/block", + CVMode: "raw", + }, + } + } else { + status.OSDs = []OSDInfo{} + for i := 0; i < numOSDsIfOnNode; i++ { + // in order to generate multiple OSDs on a node, pretend they have different configmap + // names (simply append the index). this is still deterministic. + osdID := testIDGenerator.osdID(t, fmt.Sprintf("%s-%d", cm.Name, i)) + disk := k8sutil.IndexToName(i) + status.OSDs = append(status.OSDs, OSDInfo{ + ID: osdID, + UUID: fmt.Sprintf("%032d", osdID), + BlockPath: fmt.Sprintf("/dev/vd%s", disk), + CVMode: "raw", + }) + } + } + s, _ := json.Marshal(status) + cm.Data[orchestrationStatusKey] = string(s) +} + +func setStatusConfigMapToFailed(t *testing.T, cm *corev1.ConfigMap) { + status := parseOrchestrationStatus(cm.Data) + t.Logf("updating configmap %q status to failed", cm.Name) + status.Status = OrchestrationStatusFailed + s, _ := json.Marshal(status) + cm.Data[orchestrationStatusKey] = string(s) +} + +func updateStatusConfigmap(clientset kubernetes.Interface, statusMapWatcher *watch.RaceFreeFakeWatcher, cm *corev1.ConfigMap) { + _, err := clientset.CoreV1().ConfigMaps(cm.Namespace).Update(context.TODO(), cm, metav1.UpdateOptions{}) + if err != nil { + panic(err) + } + statusMapWatcher.Modify(cm) +} diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index 2a494512b..ac3d3ac23 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/pkg/operator/ceph/cluster/osd/osd.go @@ -18,6 +18,7 @@ limitations under the License. package osd import ( + "bufio" "context" "fmt" "sort" @@ -27,34 +28,25 @@ import ( "k8s.io/client-go/kubernetes" - "github.com/banzaicloud/k8s-objectmatcher/patch" "github.com/coreos/pkg/capnslog" "github.com/pkg/errors" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" rookv1 "github.com/rook/rook/pkg/apis/rook.io/v1" "github.com/rook/rook/pkg/clusterd" - "github.com/rook/rook/pkg/daemon/ceph/client" cephclient "github.com/rook/rook/pkg/daemon/ceph/client" - kms "github.com/rook/rook/pkg/daemon/ceph/osd/kms" - "github.com/rook/rook/pkg/operator/ceph/cluster/mon" osdconfig "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" - opconfig "github.com/rook/rook/pkg/operator/ceph/config" "github.com/rook/rook/pkg/operator/ceph/controller" - opcontroller "github.com/rook/rook/pkg/operator/ceph/controller" cephver "github.com/rook/rook/pkg/operator/ceph/version" "github.com/rook/rook/pkg/operator/k8sutil" - apps "k8s.io/api/apps/v1" - batch "k8s.io/api/batch/v1" + "github.com/rook/rook/pkg/util" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/version" ) var ( logger = capnslog.NewPackageLogger("github.com/rook/rook", "op-osd") - updateDeploymentAndWait = mon.UpdateCephDeploymentAndWait cephVolumeRawEncryptionModeMinNautilusCephVersion = cephver.CephVersion{Major: 14, Minor: 2, Extra: 11} cephVolumeRawEncryptionModeMinOctopusCephVersion = cephver.CephVersion{Major: 15, Minor: 2, Extra: 5} ) @@ -130,12 +122,12 @@ type osdProperties struct { //crushHostname refers to the hostname or PVC name when the OSD is provisioned on Nodes or PVC block device, respectively. crushHostname string devices []rookv1.Device - pvc v1.PersistentVolumeClaimVolumeSource - metadataPVC v1.PersistentVolumeClaimVolumeSource - walPVC v1.PersistentVolumeClaimVolumeSource + pvc corev1.PersistentVolumeClaimVolumeSource + metadataPVC corev1.PersistentVolumeClaimVolumeSource + walPVC corev1.PersistentVolumeClaimVolumeSource pvcSize string selection rookv1.Selection - resources v1.ResourceRequirements + resources corev1.ResourceRequirements storeConfig osdconfig.StoreConfig placement rookv1.Placement preparePlacement *rookv1.Placement @@ -171,14 +163,16 @@ func (osdProps osdProperties) getPreparePlacement() rookv1.Placement { // Start the osd management func (c *Cluster) Start() error { + namespace := c.clusterInfo.Namespace config := c.newProvisionConfig() + errs := newProvisionErrors() // Validate pod's memory if specified err := controller.CheckPodMemory(cephv1.ResourcesKeyOSD, cephv1.GetOSDResources(c.spec.Resources), cephOsdPodMinimumMemory) if err != nil { return errors.Wrap(err, "failed to check pod memory") } - logger.Infof("start running osds in namespace %s", c.clusterInfo.Namespace) + logger.Infof("start running osds in namespace %q", namespace) if !c.spec.Storage.UseAllNodes && len(c.spec.Storage.Nodes) == 0 && len(c.spec.Storage.VolumeSources) == 0 && len(c.spec.Storage.StorageClassDeviceSets) == 0 { logger.Warningf("useAllNodes is set to false and no nodes, storageClassDevicesets or volumeSources are specified, no OSD pods are going to be created") @@ -191,29 +185,52 @@ func (c *Cluster) Start() error { } logger.Infof("wait timeout for healthy OSDs during upgrade or restart is %q", c.clusterInfo.OsdUpgradeTimeout) - // start the jobs to provision the OSD devices - logger.Info("start provisioning the osds on PVCs, if needed") - c.startProvisioningOverPVCs(config) + // prepare for updating existing OSDs + updateQueue, deployments, err := c.getOSDUpdateInfo(errs) + if err != nil { + return errors.Wrapf(err, "failed to get information about currently-running OSD Deployments in namespace %q", namespace) + } + logger.Debugf("%d of %d OSD Deployments need updated", updateQueue.Len(), deployments.Len()) + updateConfig := c.newUpdateConfig(config, updateQueue, deployments) - if len(config.errorMessages) > 0 { - return errors.Errorf("%d failures encountered while running osds on PVCs in namespace %q. %v", - len(config.errorMessages), c.clusterInfo.Namespace, strings.Join(config.errorMessages, "\n")) + // prepare for creating new OSDs + statusConfigMaps := util.NewSet() + + logger.Info("start provisioning the OSDs on PVCs, if needed") + pvcConfigMaps, err := c.startProvisioningOverPVCs(config, errs) + if err != nil { + return err + } + statusConfigMaps.AddSet(pvcConfigMaps) + + logger.Info("start provisioning the OSDs on nodes, if needed") + nodeConfigMaps, err := c.startProvisioningOverNodes(config, errs) + if err != nil { + return err + } + statusConfigMaps.AddSet(nodeConfigMaps.Copy()) + + createConfig := c.newCreateConfig(config, statusConfigMaps, deployments) + + // do the update and create operations + err = c.updateAndCreateOSDs(createConfig, updateConfig, errs) + if err != nil { + return errors.Wrapf(err, "failed to update/create OSDs") } - logger.Info("start provisioning the osds on nodes, if needed") - c.startProvisioningOverNodes(config) - - if len(config.errorMessages) > 0 { - return errors.Errorf("%d failures encountered while running osds on nodes in namespace %q. %v", - len(config.errorMessages), c.clusterInfo.Namespace, strings.Join(config.errorMessages, "\n")) + if errs.len() > 0 { + return errors.Errorf("%d failures encountered while running osds on nodes in namespace %q. %s", + errs.len(), namespace, errs.asMessages()) } + // clean up status configmaps that might be dangling from previous reconciles + // for example, if the storage spec changed from or a node failed in a previous failed reconcile + c.deleteAllStatusConfigMaps() + // The following block is used to apply any command(s) required by an upgrade - // The block below handles the upgrade from Mimic to Nautilus. - // This should only run before Octopus c.applyUpgradeOSDFunctionality() - logger.Infof("completed running osds in namespace %s", c.clusterInfo.Namespace) + logger.Infof("finished running OSDs in namespace %q", namespace) return nil } @@ -221,158 +238,7 @@ func (c *Cluster) shouldProvisionOverPVCs() bool { return len(c.spec.Storage.VolumeSources) > 0 || len(c.spec.Storage.StorageClassDeviceSets) > 0 } -func (c *Cluster) startProvisioningOverPVCs(config *provisionConfig) { - // Parsing storageClassDeviceSets and parsing it to volume sources - c.spec.Storage.VolumeSources = append(c.spec.Storage.VolumeSources, c.prepareStorageClassDeviceSets(config)...) - - c.ValidStorage.VolumeSources = c.spec.Storage.VolumeSources - - // no validVolumeSource is ready to run an osd - if !c.shouldProvisionOverPVCs() { - logger.Info("no volume sources defined to configure OSDs on PVCs.") - return - } - - // Check k8s version - k8sVersion, err := k8sutil.GetK8SVersion(c.context.Clientset) - if err != nil { - config.addError("error finding Kubernetes version. %v", err) - return - } - if !k8sVersion.AtLeast(version.MustParseSemantic("v1.13.0")) { - logger.Warningf("skipping OSD on PVC provisioning. Minimum Kubernetes version required: 1.13.0. Actual version: %s", k8sVersion.String()) - return - } - - existingDeployments, err := c.getExistingOSDDeploymentsOnPVCs() - if err != nil { - config.addError("failed to query existing OSD deployments on PVCs. %v", err) - return - } - - for _, volume := range c.ValidStorage.VolumeSources { - // Check whether we need to cancel the orchestration - if err := controller.CheckForCancelledOrchestration(c.context); err != nil { - config.addError("%s", err.Error()) - return - } - - dataSource, dataOK := volume.PVCSources[bluestorePVCData] - - // The data PVC template is required. - if !dataOK { - config.addError("failed to create osd for storageClassDeviceSet %q, missing the data template", volume.Name) - continue - } - - metadataSource, metadataOK := volume.PVCSources[bluestorePVCMetadata] - if metadataOK { - logger.Infof("OSD will have its main bluestore block on %q and its metadata device on %q", dataSource.ClaimName, metadataSource.ClaimName) - } else { - logger.Infof("OSD will have its main bluestore block on %q", dataSource.ClaimName) - } - - walSource, walOK := volume.PVCSources[bluestorePVCWal] - if walOK { - logger.Infof("OSD will have its wal device on %q", walSource.ClaimName) - } - - osdProps := osdProperties{ - crushHostname: dataSource.ClaimName, - pvc: dataSource, - metadataPVC: metadataSource, - walPVC: walSource, - resources: volume.Resources, - placement: volume.Placement, - preparePlacement: volume.PreparePlacement, - portable: volume.Portable, - schedulerName: volume.SchedulerName, - encrypted: volume.Encrypted, - deviceSetName: volume.Name, - } - osdProps.storeConfig.DeviceClass = volume.CrushDeviceClass - - logger.Debugf("osdProps are %+v", osdProps) - - if osdProps.encrypted { - // If the deviceSet template has "encrypted" but the Ceph version is not compatible - if !c.isCephVolumeRawModeSupported() { - errMsg := fmt.Sprintf("failed to validate storageClassDeviceSet %q. min required ceph version to support encryption is %q or %q", volume.Name, cephVolumeRawEncryptionModeMinNautilusCephVersion.String(), cephVolumeRawEncryptionModeMinOctopusCephVersion.String()) - config.addError(errMsg) - continue - } - - // create encryption Kubernetes Secret if the PVC is encrypted - key, err := generateDmCryptKey() - if err != nil { - errMsg := fmt.Sprintf("failed to generate dmcrypt key for osd claim %q. %v", osdProps.pvc.ClaimName, err) - config.addError(errMsg) - continue - } - - // Initialize the KMS code - kmsConfig := kms.NewConfig(c.context, &c.spec, c.clusterInfo) - - // We could set an env var in the Operator or a global var instead of the API call? - // Hopefully, the API is cheap and we can always retrieve the token if it has changed... - if c.spec.Security.KeyManagementService.IsTokenAuthEnabled() { - err := kms.SetTokenToEnvVar(c.context, c.spec.Security.KeyManagementService.TokenSecretName, kmsConfig.Provider, c.clusterInfo.Namespace) - if err != nil { - errMsg := fmt.Sprintf("failed to fetch kms token secret %q. %v", c.spec.Security.KeyManagementService.TokenSecretName, err) - config.addError(errMsg) - continue - } - } - - // Generate and store the encrypted key in whatever KMS is configured - err = kmsConfig.PutSecret(osdProps.pvc.ClaimName, key) - if err != nil { - errMsg := fmt.Sprintf("failed to store secret. %v", err) - config.addError(errMsg) - continue - } - } - - // Skip OSD prepare if deployment already exists for the PVC - if osdDeployment, ok := existingDeployments[dataSource.ClaimName]; ok { - logger.Infof("skip OSD prepare pod creation as OSD daemon already exists for %q", osdProps.crushHostname) - osds, err := c.getOSDInfo(osdDeployment) - if err != nil { - config.addError("failed to get osdInfo for pvc %q. %v", osdProps.crushHostname, err) - continue - } - // Update the orchestration status of this pvc to the completed state - status := OrchestrationStatus{OSDs: osds, Status: OrchestrationStatusAlreadyExists, PvcBackedOSD: true} - c.updateOSDStatus(osdProps.crushHostname, status) - continue - } - - // Update the orchestration status of this pvc to the starting state - status := OrchestrationStatus{Status: OrchestrationStatusStarting, PvcBackedOSD: true} - c.updateOSDStatus(osdProps.crushHostname, status) - - job, err := c.makeJob(osdProps, config) - if err != nil { - message := fmt.Sprintf("failed to create prepare job for pvc %s: %v", osdProps.crushHostname, err) - config.addError(message) - status := OrchestrationStatus{Status: OrchestrationStatusCompleted, Message: message, PvcBackedOSD: true} - c.updateOSDStatus(osdProps.crushHostname, status) - } - - if !c.runJob(job, osdProps.crushHostname, config, "provision") { - status := OrchestrationStatus{ - Status: OrchestrationStatusCompleted, - Message: fmt.Sprintf("failed to start osd provisioning on pvc %s", osdProps.crushHostname), - PvcBackedOSD: true, - } - c.updateOSDStatus(osdProps.crushHostname, status) - } - } - logger.Infof("start osds after provisioning is completed, if needed") - c.completeProvision(config) -} - -func (c *Cluster) getExistingOSDDeploymentsOnPVCs() (map[string]*apps.Deployment, error) { +func (c *Cluster) getExistingOSDDeploymentsOnPVCs() (*util.Set, error) { ctx := context.TODO() listOpts := metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s,%s", k8sutil.AppAttr, AppName, OSDOverPVCLabelKey)} @@ -381,232 +247,46 @@ func (c *Cluster) getExistingOSDDeploymentsOnPVCs() (map[string]*apps.Deployment return nil, errors.Wrap(err, "failed to query existing OSD deployments") } - result := map[string]*apps.Deployment{} - for i, deployment := range deployments.Items { + result := util.NewSet() + for _, deployment := range deployments.Items { if pvcID, ok := deployment.Labels[OSDOverPVCLabelKey]; ok { - result[pvcID] = &deployments.Items[i] + result.Add(pvcID) } } return result, nil } -func (c *Cluster) startProvisioningOverNodes(config *provisionConfig) { - if len(c.spec.DataDirHostPath) == 0 { - logger.Warningf("skipping osd provisioning where no dataDirHostPath is set") - return +func deploymentOnNode(c *Cluster, osd OSDInfo, nodeName string, config *provisionConfig) (*appsv1.Deployment, error) { + osdLongName := fmt.Sprintf("OSD %d on node %q", osd.ID, nodeName) + + osdProps, err := c.getOSDPropsForNode(nodeName) + if err != nil { + return nil, errors.Wrapf(err, "failed to generate config for %s", osdLongName) } - if c.spec.Storage.UseAllNodes { - if len(c.spec.Storage.Nodes) > 0 { - logger.Warningf("useAllNodes is TRUE, but nodes are specified. NODES in the cluster CR will be IGNORED unless useAllNodes is FALSE.") - } - - // Get the list of all nodes in the cluster. The placement settings will be applied below. - hostnameMap, err := k8sutil.GetNodeHostNames(c.context.Clientset) - if err != nil { - config.addError("failed to get node hostnames: %v", err) - return - } - c.spec.Storage.Nodes = nil - for _, hostname := range hostnameMap { - storageNode := rookv1.Node{ - Name: hostname, - } - c.spec.Storage.Nodes = append(c.spec.Storage.Nodes, storageNode) - } - logger.Debugf("storage nodes: %+v", c.spec.Storage.Nodes) - } - // generally speaking, this finds nodes which are capable of running new osds - validNodes := k8sutil.GetValidNodes(c.spec.Storage, c.context.Clientset, cephv1.GetOSDPlacement(c.spec.Placement)) - - logger.Infof("%d of the %d storage nodes are valid", len(validNodes), len(c.spec.Storage.Nodes)) - - c.ValidStorage = *c.spec.Storage.DeepCopy() - c.ValidStorage.Nodes = validNodes - - // no valid node is ready to run an osd - if len(validNodes) == 0 { - logger.Warningf("no valid nodes available to run osds on nodes in namespace %s", c.clusterInfo.Namespace) - return + d, err := c.makeDeployment(osdProps, osd, config) + if err != nil { + return nil, errors.Wrapf(err, "failed to generate deployment for %s", osdLongName) } - // start with nodes currently in the storage spec - for _, node := range c.ValidStorage.Nodes { - // Check whether we need to cancel the orchestration - if err := controller.CheckForCancelledOrchestration(c.context); err != nil { - config.addError("%s", err.Error()) - return - } - - // fully resolve the storage config and resources for this node - n := c.resolveNode(node.Name) - if n == nil { - logger.Warningf("node %s did not resolve", node.Name) - continue - } - - if n.Name == "" { - logger.Warningf("skipping node with a blank name! %+v", n) - continue - } - - // update the orchestration status of this node to the starting state - status := OrchestrationStatus{Status: OrchestrationStatusStarting} - c.updateOSDStatus(n.Name, status) - - // create the job that prepares osds on the node - storeConfig := osdconfig.ToStoreConfig(n.Config) - metadataDevice := osdconfig.MetadataDevice(n.Config) - osdProps := osdProperties{ - crushHostname: n.Name, - devices: n.Devices, - selection: n.Selection, - resources: n.Resources, - storeConfig: storeConfig, - metadataDevice: metadataDevice, - } - job, err := c.makeJob(osdProps, config) - if err != nil { - message := fmt.Sprintf("failed to create prepare job node %q. %v", n.Name, err) - config.addError(message) - status := OrchestrationStatus{Status: OrchestrationStatusCompleted, Message: message} - c.updateOSDStatus(n.Name, status) - } - - if !c.runJob(job, n.Name, config, "provision") { - status := OrchestrationStatus{Status: OrchestrationStatusCompleted, Message: fmt.Sprintf("failed to start osd provisioning job on node %s", n.Name)} - c.updateOSDStatus(n.Name, status) - } - } - - logger.Infof("start osds after provisioning is completed, if needed") - c.completeProvision(config) + return d, nil } -func (c *Cluster) runJob(job *batch.Job, nodeName string, config *provisionConfig, action string) bool { - if err := k8sutil.RunReplaceableJob(c.context.Clientset, job, false); err != nil { - if !kerrors.IsAlreadyExists(err) { - // we failed to create job, update the orchestration status for this node - message := fmt.Sprintf("failed to create %q job for node %q. %v", action, nodeName, err) - c.handleOrchestrationFailure(config, nodeName, message) - return false - } +func deploymentOnPVC(c *Cluster, osd OSDInfo, pvcName string, config *provisionConfig) (*appsv1.Deployment, error) { + osdLongName := fmt.Sprintf("OSD %d on PVC %q", osd.ID, pvcName) - // the job is already in progress so we will let it run to completion - } - - logger.Infof("osd %s job started for node %s", action, nodeName) - return true -} - -func (c *Cluster) startOSDDaemonsOnPVC(pvcName string, config *provisionConfig, configMap *v1.ConfigMap, status *OrchestrationStatus) { - ctx := context.TODO() - osds := status.OSDs - logger.Infof("starting %d osd daemons on pvc %s", len(osds), pvcName) osdProps, err := c.getOSDPropsForPVC(pvcName) if err != nil { - config.addError(fmt.Sprintf("%v", err)) - return + return nil, errors.Wrapf(err, "failed to generate config for %s", osdLongName) } - // start osds - for _, osd := range osds { - logger.Debugf("start osd %v", osd) - - dp, err := c.makeDeployment(osdProps, osd, config) - if err != nil { - errMsg := fmt.Sprintf("failed to create deployment for pvc %q. %v", osdProps.crushHostname, err) - config.addError(errMsg) - continue - } - - // Set the deployment hash as an annotation - err = patch.DefaultAnnotator.SetLastAppliedAnnotation(dp) - if err != nil { - errMsg := fmt.Sprintf("failed to set annotation for deployment %q. %v", dp.Name, err) - config.addError(errMsg) - continue - } - - _, createErr := c.context.Clientset.AppsV1().Deployments(c.clusterInfo.Namespace).Create(ctx, dp, metav1.CreateOptions{}) - if createErr != nil { - if kerrors.IsAlreadyExists(createErr) { - logger.Infof("deployment for osd %d already exists. updating if needed", osd.ID) - if err = updateDeploymentAndWait(c.context, c.clusterInfo, dp, opconfig.OsdType, strconv.Itoa(osd.ID), c.spec.SkipUpgradeChecks, c.spec.ContinueUpgradeAfterChecksEvenIfNotHealthy); err != nil { - logger.Errorf("failed to update osd deployment %d. %v", osd.ID, err) - } - } else { - // we failed to create job, update the orchestration status for this pvc - logger.Warningf("failed to create osd deployment for pvc %q, osd %v. %v", osdProps.pvc.ClaimName, osd, createErr) - continue - } - } - - logger.Infof("started deployment for osd %d on pvc", osd.ID) - } -} - -func (c *Cluster) startOSDDaemonsOnNode(nodeName string, config *provisionConfig, configMap *v1.ConfigMap, status *OrchestrationStatus) { - ctx := context.TODO() - osds := status.OSDs - logger.Infof("starting %d osd daemons on node %s", len(osds), nodeName) - - // fully resolve the storage config and resources for this node - n := c.resolveNode(nodeName) - if n == nil { - logger.Errorf("node %q did not resolve to start osds", nodeName) - return - } - storeConfig := osdconfig.ToStoreConfig(n.Config) - metadataDevice := osdconfig.MetadataDevice(n.Config) - - osdProps := osdProperties{ - crushHostname: n.Name, - devices: n.Devices, - selection: n.Selection, - resources: n.Resources, - storeConfig: storeConfig, - metadataDevice: metadataDevice, + d, err := c.makeDeployment(osdProps, osd, config) + if err != nil { + return nil, errors.Wrapf(err, "failed to generate deployment for %s", osdLongName) } - // start osds - for _, osd := range osds { - logger.Debugf("start osd %v", osd) - message := fmt.Sprintf("Processing OSD %d on node %q", osd.ID, nodeName) - opcontroller.UpdateCondition(c.context, c.clusterInfo.NamespacedName(), cephv1.ConditionProgressing, v1.ConditionTrue, cephv1.ClusterProgressingReason, message) - - dp, err := c.makeDeployment(osdProps, osd, config) - if err != nil { - errMsg := fmt.Sprintf("failed to create deployment for node %s: %v", n.Name, err) - config.addError(errMsg) - continue - } - - // Set the deployment hash as an annotation - err = patch.DefaultAnnotator.SetLastAppliedAnnotation(dp) - if err != nil { - errMsg := fmt.Sprintf("failed to set annotation for deployment %q. %v", dp.Name, err) - config.addError(errMsg) - continue - } - - _, createErr := c.context.Clientset.AppsV1().Deployments(c.clusterInfo.Namespace).Create(ctx, dp, metav1.CreateOptions{}) - if createErr != nil { - if kerrors.IsAlreadyExists(createErr) { - logger.Debugf("deployment for osd %d already exists. updating if needed", osd.ID) - if err = updateDeploymentAndWait(c.context, c.clusterInfo, dp, opconfig.OsdType, strconv.Itoa(osd.ID), c.spec.SkipUpgradeChecks, c.spec.ContinueUpgradeAfterChecksEvenIfNotHealthy); err != nil { - logger.Errorf("failed to update osd deployment %d. %v", osd.ID, err) - } - } else { - // we failed to create job, update the orchestration status for this pvc - logger.Warningf("failed to create osd deployment for node %q, osd %+v. %v", n.Name, osd, createErr) - continue - } - } else { - logger.Infof("created deployment for osd %d", osd.ID) - } - } + return d, nil } func (c *Cluster) resolveNode(nodeName string) *rookv1.Node { @@ -620,6 +300,27 @@ func (c *Cluster) resolveNode(nodeName string) *rookv1.Node { return rookNode } +func (c *Cluster) getOSDPropsForNode(nodeName string) (osdProperties, error) { + // fully resolve the storage config and resources for this node + n := c.resolveNode(nodeName) + if n == nil { + return osdProperties{}, errors.Errorf("failed to resolve node %q", nodeName) + } + + storeConfig := osdconfig.ToStoreConfig(n.Config) + metadataDevice := osdconfig.MetadataDevice(n.Config) + osdProps := osdProperties{ + crushHostname: n.Name, + devices: n.Devices, + selection: n.Selection, + resources: n.Resources, + storeConfig: storeConfig, + metadataDevice: metadataDevice, + } + + return osdProps, nil +} + func (c *Cluster) getOSDPropsForPVC(pvcName string) (osdProperties, error) { for _, volumeSource := range c.ValidStorage.VolumeSources { // The data PVC template is required. @@ -668,13 +369,13 @@ func (c *Cluster) getOSDPropsForPVC(pvcName string) (osdProperties, error) { var err error osdProps.crushHostname, err = c.getPVCHostName(pvcName) if err != nil { - return osdProperties{}, errors.Wrapf(err, "Unable to get crushHostname of non portable pvc %s", pvcName) + return osdProperties{}, errors.Wrapf(err, "failed to get crushHostname of non-portable PVC %q", pvcName) } } return osdProps, nil } } - return osdProperties{}, errors.Errorf("no valid VolumeSource found for pvc %s", pvcName) + return osdProperties{}, errors.Errorf("failed to find valid VolumeSource for PVC %q", pvcName) } // getPVCHostName finds the node where an OSD pod should be assigned with a node selector. @@ -693,7 +394,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 == v1.LabelHostname { + if label == corev1.LabelHostname { return value, nil } } @@ -720,20 +421,34 @@ func (c *Cluster) getPVCHostName(pvcName string) (string, error) { return "", errors.Errorf("node selector not found on deployment for osd with pvc %q", pvcName) } -func (c *Cluster) getOSDInfo(d *apps.Deployment) ([]OSDInfo, error) { +func getOSDID(d *appsv1.Deployment) (int, error) { + osdID, err := strconv.Atoi(d.Labels[OsdIdLabelKey]) + if err != nil { + // add a question to the user AFTER the error text to help them recover from user error + return -1, errors.Wrapf(err, "failed to parse label \"ceph-osd-id\" on deployment %q. did a user modify the deployment and remove the label?", d.Name) + } + return osdID, nil +} + +func (c *Cluster) getOSDInfo(d *appsv1.Deployment) (OSDInfo, error) { container := d.Spec.Template.Spec.Containers[0] var osd OSDInfo - osdID, err := strconv.Atoi(d.Labels[OsdIdLabelKey]) + osdID, err := getOSDID(d) if err != nil { - return []OSDInfo{}, errors.Wrap(err, "error parsing ceph-osd-id") + return OSDInfo{}, err } osd.ID = osdID + isPVC := false + for _, envVar := range d.Spec.Template.Spec.Containers[0].Env { if envVar.Name == "ROOK_OSD_UUID" { osd.UUID = envVar.Value } + if envVar.Name == "ROOK_PVC_BACKED_OSD" { + isPVC = true + } if envVar.Name == "ROOK_BLOCK_PATH" || envVar.Name == "ROOK_LV_PATH" { osd.BlockPath = envVar.Value } @@ -746,7 +461,7 @@ func (c *Cluster) getOSDInfo(d *apps.Deployment) ([]OSDInfo, error) { if envVar.Name == "ROOK_LV_BACKED_PV" { lvBackedPV, err := strconv.ParseBool(envVar.Value) if err != nil { - return []OSDInfo{}, errors.Wrap(err, "error parsing ROOK_LV_BACKED_PV") + return OSDInfo{}, errors.Wrap(err, "failed to parse ROOK_LV_BACKED_PV") } osd.LVBackedPV = lvBackedPV } @@ -758,9 +473,19 @@ func (c *Cluster) getOSDInfo(d *apps.Deployment) ([]OSDInfo, error) { } } + // Needed for upgrade from v1.5 to v1.6. Rook v1.5 did not set ROOK_BLOCK_PATH for OSDs on nodes + // where the 'activate' init container was needed. + if !isPVC && osd.BlockPath == "" { + osd.BlockPath, err = getBlockPathFromActivateInitContainer(d) + if err != nil { + return OSDInfo{}, errors.Wrapf(err, "failed to extract legacy OSD block path from deployment %q", d.Name) + } + } + // If CVMode is empty, this likely means we upgraded Rook // This property did not exist before so we need to initialize it if osd.CVMode == "" { + logger.Infof("required CVMode for OSD %d was not found. assuming this is an LVM OSD", osd.ID) osd.CVMode = "lvm" } @@ -776,7 +501,7 @@ func (c *Cluster) getOSDInfo(d *apps.Deployment) ([]OSDInfo, error) { } if !locationFound { - location, _, err := getLocationFromPod(c.context.Clientset, d, client.GetCrushRootFromSpec(&c.spec)) + location, _, err := getLocationFromPod(c.context.Clientset, d, cephclient.GetCrushRootFromSpec(&c.spec)) if err != nil { logger.Errorf("failed to get location. %v", err) } else { @@ -785,13 +510,59 @@ func (c *Cluster) getOSDInfo(d *apps.Deployment) ([]OSDInfo, error) { } if osd.UUID == "" || osd.BlockPath == "" { - return []OSDInfo{}, errors.Errorf("failed to get required osdInfo. %+v", osd) + return OSDInfo{}, errors.Errorf("failed to get required osdInfo. %+v", osd) } - return []OSDInfo{osd}, nil + return osd, nil } -func getLocationFromPod(clientset kubernetes.Interface, d *apps.Deployment, crushRoot string) (string, string, error) { +func osdIsOnPVC(d *appsv1.Deployment) bool { + if _, ok := d.Labels[OSDOverPVCLabelKey]; ok { + return true + } + return false +} + +func getNodeOrPVCName(d *appsv1.Deployment) (string, error) { + if v, ok := d.Labels[OSDOverPVCLabelKey]; ok { + return v, nil // OSD is on PVC + } + for k, v := range d.Spec.Template.Spec.NodeSelector { + if k == corev1.LabelHostname { + return v, nil + } + } + return "", errors.Errorf("failed to find node/PVC name for OSD deployment %q: %+v", d.Name, d) +} + +// Needed for upgrades from v1.5 to v1.6 +func getBlockPathFromActivateInitContainer(d *appsv1.Deployment) (string, error) { + initContainers := d.Spec.Template.Spec.InitContainers + for _, c := range initContainers { + if c.Name != activatePVCOSDInitContainer { + continue + } + if len(c.Command) != 3 { + return "", errors.Errorf("activate init container has fewer command arguments (%d) than expected (3)", len(c.Command)) + } + script := c.Command[2] + varAssignment := "DEVICE=" // this variable assignment is followed by the block path + scanner := bufio.NewScanner(strings.NewReader(script)) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if strings.HasPrefix(line, varAssignment) { + device := strings.TrimPrefix(line, varAssignment) + return device, nil + } + } + if scanner.Err() != nil { + return "", errors.Wrapf(scanner.Err(), "failed to scan through activate init script for variable assignment %q", varAssignment) + } + } + return "", errors.Errorf("failed to find activate init container") +} + +func getLocationFromPod(clientset kubernetes.Interface, d *appsv1.Deployment, crushRoot string) (string, string, error) { ctx := context.TODO() pods, err := clientset.CoreV1().Pods(d.Namespace).List(ctx, metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", OsdIdLabelKey, d.Labels[OsdIdLabelKey])}) if err != nil || len(pods.Items) == 0 { @@ -817,7 +588,6 @@ func getLocationFromPod(clientset kubernetes.Interface, d *apps.Deployment, crus // topologyAffinity: The label to be applied to the OSD daemon to guarantee it will start in the same // topology as the OSD prepare job. func GetLocationWithNode(clientset kubernetes.Interface, nodeName string, crushRoot, crushHostname string) (string, string, error) { - node, err := getNode(clientset, nodeName) if err != nil { return "", "", errors.Wrapf(err, "could not get the node for topology labels") @@ -834,7 +604,7 @@ func GetLocationWithNode(clientset kubernetes.Interface, nodeName string, crushR // Start with the host name in the CRUSH map // Keep the fully qualified host name in the crush map, but replace the dots with dashes to satisfy ceph - hostName := client.NormalizeCrushName(crushHostname) + hostName := cephclient.NormalizeCrushName(crushHostname) locArgs := []string{fmt.Sprintf("root=%s", crushRoot), fmt.Sprintf("host=%s", hostName)} nodeLabels := node.GetLabels() @@ -868,7 +638,6 @@ func getNode(clientset kubernetes.Interface, nodeName string) (*corev1.Node, err } func updateLocationWithNodeLabels(location *[]string, nodeLabels map[string]string) string { - topology, topologyAffinity := ExtractOSDTopologyFromLabels(nodeLabels) keys := make([]string, 0, len(topology)) @@ -879,7 +648,7 @@ func updateLocationWithNodeLabels(location *[]string, nodeLabels map[string]stri for _, topologyType := range keys { if topologyType != "host" { - client.UpdateCrushMapValue(location, topologyType, topology[topologyType]) + cephclient.UpdateCrushMapValue(location, topologyType, topology[topologyType]) } } return topologyAffinity @@ -889,7 +658,7 @@ func (c *Cluster) applyUpgradeOSDFunctionality() { var osdVersion *cephver.CephVersion // Get all the daemons versions - versions, err := client.GetAllCephDaemonVersions(c.context, c.clusterInfo) + versions, err := cephclient.GetAllCephDaemonVersions(c.context, c.clusterInfo) if err != nil { logger.Warningf("failed to get ceph daemons versions; this likely means there are no osds yet. %v", err) return @@ -909,7 +678,7 @@ func (c *Cluster) applyUpgradeOSDFunctionality() { } // if the version of these OSDs is Octopus then we run the command if osdVersion.IsOctopus() { - err = client.EnableReleaseOSDFunctionality(c.context, c.clusterInfo, "octopus") + err = cephclient.EnableReleaseOSDFunctionality(c.context, c.clusterInfo, "octopus") if err != nil { logger.Warningf("failed to enable new osd functionality. %v", err) return diff --git a/pkg/operator/ceph/cluster/osd/osd_pvc_test.go b/pkg/operator/ceph/cluster/osd/osd_pvc_test.go deleted file mode 100644 index 324ee4da4..000000000 --- a/pkg/operator/ceph/cluster/osd/osd_pvc_test.go +++ /dev/null @@ -1,680 +0,0 @@ -/* -Copyright 2021 The Rook Authors. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package osd - -import ( - "context" - "encoding/json" - "fmt" - "os" - "strconv" - "strings" - "sync" - "testing" - "time" - - "github.com/coreos/pkg/capnslog" - "github.com/pkg/errors" - cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" - rookv1 "github.com/rook/rook/pkg/apis/rook.io/v1" - "github.com/rook/rook/pkg/clusterd" - cephclient "github.com/rook/rook/pkg/daemon/ceph/client" - cephver "github.com/rook/rook/pkg/operator/ceph/version" - "github.com/rook/rook/pkg/operator/k8sutil" - "github.com/rook/rook/pkg/operator/test" - exectest "github.com/rook/rook/pkg/util/exec/test" - "github.com/stretchr/testify/assert" - "github.com/tevino/abool" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/watch" - "k8s.io/client-go/kubernetes/fake" - k8stesting "k8s.io/client-go/testing" -) - -// The definition for this test is a wrapper for the test function that adds a timeout -func TestOSDsOnPVC(t *testing.T) { - oldLogger := *logger - defer func() { logger = &oldLogger }() // reset logger to default after this test - logger.SetLevel(capnslog.TRACE) // want more log info for this test if it fails - - oldOpportunisticDuration := osdOpportunisticUpdateDuration - defer func() { osdOpportunisticUpdateDuration = oldOpportunisticDuration }() - // lower the opportunistic update check duration for unit tests to speed them up - osdOpportunisticUpdateDuration = 1 * time.Millisecond - - // This test runs in less than 150 milliseconds on a 6-core CPU w/ 16GB RAM - // GitHub CI runner can be a lot slower (~2.5 seconds) - done := make(chan bool) - timeout := time.After(50 * 150 * time.Millisecond) - - go func() { - // use defer because t.Fatal will kill this goroutine, and we always want done set if the - // func stops running - defer func() { done <- true }() - // run the actual test - testOSDsOnPVC(t) - }() - - select { - case <-timeout: - t.Fatal("Test timed out. This is a test failure.") - case <-done: - } -} - -// This is the actual test. If it hangs, we should consider that an error. Writing a timeout in -// tests requires running the test in a goroutine, so there is a timeout wrapper above -func testOSDsOnPVC(t *testing.T) { - namespace := "osd-on-pvc" - ctx := context.TODO() - // we don't need to create actual nodes for this test, but this is the set of nodes which should - // we will use to create fake placements for OSD prepare job pods - osdIDGenerator := newOSDIDGenerator() - - // set up a fake k8s client set and watcher to generate events that the operator will listen to - clientset := test.NewComplexClientset(t) - test.AddSomeReadyNodes(t, clientset, 3) - assignPodToNode := true - test.PrependComplexJobReactor(t, clientset, assignPodToNode) - test.SetFakeKubernetesVersion(clientset, "v1.13.2") // v1.13 or higher is required for OSDs on PVC - - os.Setenv(k8sutil.PodNamespaceEnvVar, namespace) - defer os.Unsetenv(k8sutil.PodNamespaceEnvVar) - - statusMapWatcher := watch.NewRaceFreeFake() - clientset.PrependWatchReactor("configmaps", k8stesting.DefaultWatchReactor(statusMapWatcher, nil)) - - // Helper methods to set "completed" status on "starting" ConfigMaps. - setStatusConfigMapToCompleted := func(cm *corev1.ConfigMap) { - // can't use mockNodeOrchestrationCompleted here b/c it uses a context.CoreV1() method that - // is mutex locked when a reactor is processing - status := parseOrchestrationStatus(cm.Data) - t.Logf("configmap reactor: updating configmap %q status to completed", cm.Name) - // configmap names are deterministic can be mapped indirectly to an OSD ID, and since the - // configmaps are used to report completion status of OSD provisioning, we use this property in - // thse unit tests - osdID := osdIDGenerator.osdID(t, cm.Name) - status.Status = OrchestrationStatusCompleted - status.PvcBackedOSD = true - status.OSDs = []OSDInfo{ - { - ID: osdID, - UUID: fmt.Sprintf("%032d", osdID), - BlockPath: "/dev/path/to/block", - }, - } - s, _ := json.Marshal(status) - cm.Data[orchestrationStatusKey] = string(s) - } - - createConfigMapWithStatusStartingCallback := func(cm *corev1.ConfigMap) { - // placeholder to be defined later in tests - } - deleteConfigMapWithStatusAlreadyExistsCallback := func(cm *corev1.ConfigMap, action k8stesting.DeleteActionImpl) { - // placeholder to be defined later in tests - } - - var cmReactor k8stesting.ReactionFunc = func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - switch action := action.(type) { - case k8stesting.CreateActionImpl: - obj := action.GetObject() - cm, ok := obj.(*corev1.ConfigMap) - if !ok { - t.Fatal("err! action not a configmap") - } - - status := parseOrchestrationStatus(cm.Data) - if status.Status == OrchestrationStatusStarting { - // allow tests to specify a custom callback for this case - createConfigMapWithStatusStartingCallback(cm) - } - - case k8stesting.DeleteActionImpl: - // get the CM being deleted to figure out some info about it - obj, err := clientset.Tracker().Get(action.GetResource(), action.GetNamespace(), action.Name) - if err != nil { - t.Fatalf("err! could not get configmap %q", action.Name) - } - cm, ok := obj.(*corev1.ConfigMap) - if !ok { - t.Fatal("err! action not a configmap") - } - - status := parseOrchestrationStatus(cm.Data) - if status.Status == OrchestrationStatusAlreadyExists { - t.Logf("configmap reactor: delete: OSD for configmap %q was updated", cm.Name) - // allow tests to specify a custom callback for this case - deleteConfigMapWithStatusAlreadyExistsCallback(cm, action) - } else if status.Status == OrchestrationStatusCompleted { - t.Logf("configmap reactor: delete: OSD for configmap %q was created", cm.Name) - } - } - - // modify it in-place and allow it to be created later with these changes - return false, nil, nil - } - clientset.PrependReactor("*", "configmaps", cmReactor) - - deploymentOps := newResourceOperationList() - - // make a very simple reactor to record when deployments were created - var deploymentReactor k8stesting.ReactionFunc = func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - createAction, ok := action.(k8stesting.CreateAction) - if !ok { - t.Fatal("err! action is not a create action") - return false, nil, nil - } - obj := createAction.GetObject() - d, ok := obj.(*appsv1.Deployment) - if !ok { - t.Fatal("err! action not a deployment") - return false, nil, nil - } - if o, _ := clientset.Tracker().Get(action.GetResource(), d.Namespace, d.Name); o != nil { - // deployment already exists, so this isn't be a valid create - return false, nil, nil - } - t.Logf("creating deployment %q", d.Name) - deploymentOps.Add(d.Name, "create") - return false, nil, nil - } - clientset.PrependReactor("create", "deployments", deploymentReactor) - - // patch the updateDeploymentAndWait function to always report success and record when - // deployments are updated - oldUDAW := updateDeploymentAndWait - defer func() { - updateDeploymentAndWait = oldUDAW - }() - updateDeploymentAndWait = func(context *clusterd.Context, clusterInfo *cephclient.ClusterInfo, deployment *appsv1.Deployment, daemonType string, daemonName string, skipUpgradeChecks bool, continueUpgradeAfterChecksEvenIfNotHealthy bool) error { - t.Logf("updating deployment %q", deployment.Name) - deploymentOps.Add(deployment.Name, "update") - return nil - } - - // wait for a number of deployments to be updated - waitForDeploymentOps := func(count int) { - for { - if deploymentOps.Len() >= count { - return - } - <-time.After(1 * time.Millisecond) - } - } - - clusterInfo := &cephclient.ClusterInfo{ - Namespace: namespace, - CephVersion: cephver.Nautilus, - } - clusterInfo.SetName("mycluster") - clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) - executor := osdPVCTestExecutor(t, clientset, namespace) - - context := &clusterd.Context{Clientset: clientset, ConfigDir: "/var/lib/rook", Executor: executor, RequestCancelOrchestration: abool.New()} - storageClassName := "test-storage-class" - volumeMode := corev1.PersistentVolumeBlock - spec := cephv1.ClusterSpec{ - CephVersion: cephv1.CephVersionSpec{ - Image: "ceph/ceph:v14.2.2", - }, - DataDirHostPath: context.ConfigDir, - Storage: rookv1.StorageScopeSpec{ - StorageClassDeviceSets: []rookv1.StorageClassDeviceSet{ - { - Name: "set1", - Count: 5, - VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "data", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("10Gi"), - }, - }, - StorageClassName: &storageClassName, - VolumeMode: &volumeMode, - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - }, - }, - }, - }, - }, - }, - } - - // ============================================================================================= - t.Log("Step 1: create new PVCs") - - // when creating new configmaps with status "starting", simulate them becoming "completed" - // before any opportunistic updates can happen by changing the status to "completed" before - // the config map gets created by the fake k8s clientset. - createConfigMapWithStatusStartingCallback = func(cm *corev1.ConfigMap) { - setStatusConfigMapToCompleted(cm) - } - deleteConfigMapWithStatusAlreadyExistsCallback = func(cm *corev1.ConfigMap, action k8stesting.DeleteActionImpl) { - // do nothing on CM deletes - } - - var c *Cluster - var provisionDone bool = false - waitForDone := func() { - for { - if provisionDone { - t.Log("provisioning done") - break - } - t.Log("provisioning not done. waiting...") - time.Sleep(time.Millisecond) - } - } - run := func() { - // reset - deploymentOps = newResourceOperationList() - statusMapWatcher.Reset() - - // kick off the start of the orchestration in a goroutine so we can watch the results - // and manipulate confimaps in the test if needed - c = New(context, clusterInfo, spec, "myversion") - provisionDone = false - go func() { - provisionConfig := c.newProvisionConfig() - c.startProvisioningOverPVCs(provisionConfig) - provisionDone = true - }() - } - run() - - numExpectedPVCs := 5 - waitForNumPVCs(t, clientset, namespace, numExpectedPVCs) - - waitForNumDeployments(t, clientset, namespace, numExpectedPVCs) - // 5 deployments should have been created - assert.Equal(t, 5, deploymentOps.Len()) - // all 5 should be create operations - assert.Len(t, deploymentOps.ResourcesWithOperation("create"), 5) - t.Log("deployments successfully created for new PVCs") - - waitForDone() - - // ============================================================================================= - t.Log("Step 2: verify deployments are updated when run again") - // clean the create times maps - reset := func() { - deploymentOps = newResourceOperationList() - // fake 'watcher' can close the channel for long tests, so reset when we can - statusMapWatcher.Reset() - } - reset() - - run() - - waitForNumPVCs(t, clientset, namespace, numExpectedPVCs) - - // 5 deployments should have been operated on - waitForDeploymentOps(numExpectedPVCs) - // all 5 should be update operations - updatedDeployments := deploymentOps.ResourcesWithOperation("update") - assert.Len(t, updatedDeployments, 5) - - // use later to ensure existing deployments are updated - existingDeployments := updatedDeployments - - waitForDone() - - // ============================================================================================= - t.Log("Step 3: verify new deployments are created before existing ones are updated") - reset() - - spec.Storage.StorageClassDeviceSets[0].Count = 8 - numExpectedPVCs = 8 - - run() - - waitForNumPVCs(t, clientset, namespace, numExpectedPVCs) - waitForNumDeployments(t, clientset, namespace, numExpectedPVCs) - waitForDeploymentOps(numExpectedPVCs) - - // the same deployments from before should be updated here also - updatedDeployments = deploymentOps.ResourcesWithOperation("update") - assert.Len(t, updatedDeployments, 5) - assert.ElementsMatch(t, existingDeployments, updatedDeployments) - - createdDeployments := deploymentOps.ResourcesWithOperation("create") - assert.Len(t, createdDeployments, 3) - - for i, do := range deploymentOps.List() { - if i < 3 { - // first 3 ops should be create ops - assert.Equal(t, "create", do.operation) - } else { - // final 5 ops should be update ops - assert.Equal(t, "update", do.operation) - } - } - - existingDeployments = append(createdDeployments, updatedDeployments...) - - waitForDone() - - // ============================================================================================= - t.Log("Step 4: verify updates can happen opportunistically") - reset() - - spec.Storage.StorageClassDeviceSets[0].Count = 10 - numExpectedPVCs = 10 - - // In this test we carefully control the configmaps. When a configmap with status - // "alreadyExisting" is deleted, we know an OSD deployment just finished updating. We then - // immediately set one of the configmaps in "starting" state to "completed" so that it should - // be the next status configmap to be processed; a new OSD should be created for it. We - // therefore know that the first operation should be an update and the second a create. Then on - // in update-then-create fashion until all creates are done, followed by all updates. - configMapsThatNeedUpdatedToCompleted := []string{} - createConfigMapWithStatusStartingCallback = func(cm *corev1.ConfigMap) { - t.Logf("configmap reactor: create: marking that configmap %q needs to be completed later", cm.Name) - configMapsThatNeedUpdatedToCompleted = append(configMapsThatNeedUpdatedToCompleted, cm.Name) - } - deleteConfigMapWithStatusAlreadyExistsCallback = func(cm *corev1.ConfigMap, action k8stesting.DeleteActionImpl) { - if len(configMapsThatNeedUpdatedToCompleted) > 0 { - cmName := configMapsThatNeedUpdatedToCompleted[0] - obj, err := clientset.Tracker().Get(action.GetResource(), action.GetNamespace(), cmName) - if err != nil { - t.Fatalf("err! could not get configmap %q", cmName) - } - cm, ok := obj.(*corev1.ConfigMap) - if !ok { - t.Fatal("err! action not a configmap") - } - setStatusConfigMapToCompleted(cm) - err = clientset.Tracker().Update(action.GetResource(), cm, action.GetNamespace()) - if err != nil { - t.Fatalf("err! failed to update configmap to completed. %v", err) - } - statusMapWatcher.Modify(cm) // MUST inform the fake watcher we made a change - configMapsThatNeedUpdatedToCompleted = configMapsThatNeedUpdatedToCompleted[1:] - } - } - - run() - - waitForNumPVCs(t, clientset, namespace, numExpectedPVCs) - waitForNumDeployments(t, clientset, namespace, numExpectedPVCs) - waitForDeploymentOps(numExpectedPVCs) - - updatedDeployments = deploymentOps.ResourcesWithOperation("update") - assert.Len(t, updatedDeployments, 8) - assert.ElementsMatch(t, existingDeployments, updatedDeployments) - - createdDeployments = deploymentOps.ResourcesWithOperation("create") - assert.Len(t, createdDeployments, 2) - - assert.Equal(t, - []string{ - "update", - "create", - "update", - "create", - "update", - "update", - "update", - "update", - "update", - "update", - }, deploymentOps.OperationsInOrder()) - - existingDeployments = append(createdDeployments, updatedDeployments...) - - waitForDone() - - // ============================================================================================= - t.Log("Step 5: verify opportunistic updates can all happen before creates") - reset() - - spec.Storage.StorageClassDeviceSets[0].Count = 12 - numExpectedPVCs = 12 - - // In this test, we stop all configmaps from being moved from "starting" to "completed" status - // in the configmap reactor so that all opportunistic updates should happen before new OSDs - // get created. - configMapsThatNeedUpdatedToCompleted = []string{} - createConfigMapWithStatusStartingCallback = func(cm *corev1.ConfigMap) { - // re-define this behavior as a reminder for readers of the test - t.Logf("configmap reactor: create: marking that configmap %q needs to be completed later", cm.Name) - configMapsThatNeedUpdatedToCompleted = append(configMapsThatNeedUpdatedToCompleted, cm.Name) - } - deleteConfigMapWithStatusAlreadyExistsCallback = func(cm *corev1.ConfigMap, action k8stesting.DeleteActionImpl) { - // do NOT automatically move configmaps from "starting" to "completed" - } - - run() - - waitForDeploymentOps(10) // wait for 10 updates - - updatedDeployments = deploymentOps.ResourcesWithOperation("update") - assert.Len(t, updatedDeployments, 10) - assert.ElementsMatch(t, existingDeployments, updatedDeployments) - - // update configmaps from "starting" to "completed" - for _, cmName := range configMapsThatNeedUpdatedToCompleted { - cm, err := clientset.CoreV1().ConfigMaps(namespace).Get(ctx, cmName, metav1.GetOptions{}) - assert.NoError(t, err) - setStatusConfigMapToCompleted(cm) - cm, err = clientset.CoreV1().ConfigMaps(namespace).Update(ctx, cm, metav1.UpdateOptions{}) - assert.NoError(t, err) - statusMapWatcher.Modify(cm) // MUST inform the fake watcher we made a change - } - - waitForNumPVCs(t, clientset, namespace, numExpectedPVCs) - waitForNumDeployments(t, clientset, namespace, numExpectedPVCs) - waitForDeploymentOps(numExpectedPVCs) - - // should be 2 more create operations - createdDeployments = deploymentOps.ResourcesWithOperation("create") - assert.Len(t, createdDeployments, 2) - - for i, do := range deploymentOps.List() { - if i < 10 { - // first 10 ops should be update ops - assert.Equal(t, "update", do.operation) - } else { - // final 2 ops should be update ops - assert.Equal(t, "create", do.operation) - } - } - - waitForDone() - t.Log("success") -} - -/* - * mock executor to handle ceph commands - */ - -func osdPVCTestExecutor(t *testing.T, clientset *fake.Clientset, namespace string) *exectest.MockExecutor { - return &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { - t.Logf("command: %s %v", command, args) - if command != "ceph" { - return "", errors.Errorf("unexpected command %q with args %v", command, args) - } - if args[0] == "auth" { - if args[1] == "get-or-create-key" { - return "{\"key\": \"does-not-matter\"}", nil - } - } - if args[0] == "osd" { - if args[1] == "ok-to-stop" { - return "", nil // no need to return text, only output status is used - } - if args[1] == "ls" { - // ceph osd ls returns an array of osd IDs like [0,1,2] - // build this based on the number of deployments since they should be equal - // for this test - l, err := clientset.AppsV1().Deployments(namespace).List(context.TODO(), metav1.ListOptions{}) - if err != nil { - t.Fatalf("failed to build 'ceph osd ls' output. %v", err) - } - num := len(l.Items) - a := []string{} - for i := 0; i < num; i++ { - a = append(a, strconv.Itoa(i)) - } - return fmt.Sprintf("[%s]", strings.Join(a, ",")), nil - } - if args[1] == "tree" { - return `{"nodes":[{"id":-1,"name":"default","type":"root","type_id":11,"children":[-3]},{"id":-3,"name":"master","type":"host","type_id":1,"pool_weights":{},"children":[2,1,0]},{"id":0,"device_class":"hdd","name":"osd.0","type":"osd","type_id":0,"crush_weight":0.009796142578125,"depth":2,"pool_weights":{},"exists":1,"status":"up","reweight":1,"primary_affinity":1},{"id":1,"device_class":"hdd","name":"osd.1","type":"osd","type_id":0,"crush_weight":0.009796142578125,"depth":2,"pool_weights":{},"exists":1,"status":"up","reweight":1,"primary_affinity":1},{"id":2,"device_class":"hdd","name":"osd.2","type":"osd","type_id":0,"crush_weight":0.009796142578125,"depth":2,"pool_weights":{},"exists":1,"status":"up","reweight":1,"primary_affinity":1}],"stray":[]}`, nil - } - } - if args[0] == "versions" { - // the update deploy code only cares about the mons from the ceph version command results - v := `{"mon":{"ceph version 14.2.2 (somehash) nautilus (stable)":3}}` - return v, nil - } - return "", errors.Errorf("unexpected ceph command %q", args) - }, - } -} - -/* - * basic helper functions - */ - -// node names for OSDs on PVC end up being the name of the PVC -func waitForNumPVCs(t *testing.T, clientset *fake.Clientset, namespace string, count int) { - for { - l, err := clientset.CoreV1().PersistentVolumeClaims(namespace).List(context.TODO(), metav1.ListOptions{}) - assert.NoError(t, err) - if len(l.Items) >= count { - t.Log("PVCs for OSDs on PVC all exist") - break - } - <-time.After(1 * time.Millisecond) - } -} - -func waitForNumDeployments(t *testing.T, clientset *fake.Clientset, namespace string, count int) { - for { - l, err := clientset.AppsV1().Deployments(namespace).List(context.TODO(), metav1.ListOptions{}) - assert.NoError(t, err) - if len(l.Items) >= count { - t.Log("Deployments for OSDs on PVC all exist") - break - } - <-time.After(1 * time.Millisecond) - } -} - -/* - * Unique and consistent OSD ID generator - */ - -type osdIDGenerator struct { - nextOSDID int - osdIDMap map[string]int -} - -func newOSDIDGenerator() osdIDGenerator { - return osdIDGenerator{ - nextOSDID: 0, - osdIDMap: map[string]int{}, - } -} - -func (g *osdIDGenerator) osdID(t *testing.T, namedResource string) int { - if id, ok := g.osdIDMap[namedResource]; ok { - t.Logf("resource %q has existing OSD ID %d", namedResource, id) - return id - } - id := g.nextOSDID - g.osdIDMap[namedResource] = id - g.nextOSDID++ - t.Logf("generated new OSD ID %d for resource %q", id, namedResource) - return id -} - -/* - * resourceOperationList - * We want to keep track of the order in which some resources (notably deployments) are created and - * updated, and the tracker should be thread safe since there could be operations occurring in - * parallel. - */ - -type resourceOperation struct { - resourceName string - operation string // e.g., "create", "update" -} - -func newResourceOperation(resourceName, operation string) resourceOperation { - return resourceOperation{resourceName, operation} -} - -type resourceOperationList struct { - sync.Mutex - resourceOps []resourceOperation -} - -func newResourceOperationList() *resourceOperationList { - return &resourceOperationList{ - sync.Mutex{}, - []resourceOperation{}, - } -} - -func (r *resourceOperationList) Add(resourceName, operation string) { - r.Lock() - defer r.Unlock() - r.resourceOps = append(r.resourceOps, newResourceOperation(resourceName, operation)) -} - -func (r *resourceOperationList) Len() int { - r.Lock() - defer r.Unlock() - return len(r.resourceOps) -} - -func (r *resourceOperationList) List() []resourceOperation { - return r.resourceOps -} - -// Return only the resources which have a given operation -func (r *resourceOperationList) ResourcesWithOperation(operation string) []string { - resources := []string{} - for _, ro := range r.List() { - if ro.operation == operation { - resources = append(resources, ro.resourceName) - } - } - return resources -} - -// Return all operations in order without resource names -func (r *resourceOperationList) OperationsInOrder() []string { - ops := []string{} - for _, ro := range r.List() { - ops = append(ops, ro.operation) - } - return ops -} diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index 96a1126a4..3d77e60d0 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/pkg/operator/ceph/cluster/osd/osd_test.go @@ -28,19 +28,22 @@ import ( "github.com/rook/rook/pkg/clusterd" "github.com/rook/rook/pkg/daemon/ceph/client" cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + cephclientfake "github.com/rook/rook/pkg/daemon/ceph/client/fake" discoverDaemon "github.com/rook/rook/pkg/daemon/discover" "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" opconfig "github.com/rook/rook/pkg/operator/ceph/config" cephver "github.com/rook/rook/pkg/operator/ceph/version" "github.com/rook/rook/pkg/operator/k8sutil" + "github.com/rook/rook/pkg/operator/test" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" "github.com/tevino/abool" apps "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" @@ -48,10 +51,10 @@ import ( func TestOSDProperties(t *testing.T) { osdProps := []osdProperties{ - {pvc: v1.PersistentVolumeClaimVolumeSource{ClaimName: "claim"}, - metadataPVC: v1.PersistentVolumeClaimVolumeSource{ClaimName: "claim"}}, - {pvc: v1.PersistentVolumeClaimVolumeSource{ClaimName: ""}, - metadataPVC: v1.PersistentVolumeClaimVolumeSource{ClaimName: ""}}, + {pvc: corev1.PersistentVolumeClaimVolumeSource{ClaimName: "claim"}, + metadataPVC: corev1.PersistentVolumeClaimVolumeSource{ClaimName: "claim"}}, + {pvc: corev1.PersistentVolumeClaimVolumeSource{ClaimName: ""}, + metadataPVC: corev1.PersistentVolumeClaimVolumeSource{ClaimName: ""}}, } expected := [][2]bool{ {true, true}, @@ -86,7 +89,7 @@ func createDiscoverConfigmap(nodeName, ns string, clientset *fake.Clientset) err ctx := context.TODO() data := make(map[string]string, 1) data[discoverDaemon.LocalDiskCMData] = `[{"name":"sdx","parent":"","hasChildren":false,"devLinks":"/dev/disk/by-id/scsi-36001405f826bd553d8c4dbf9f41c18be /dev/disk/by-id/wwn-0x6001405f826bd553d8c4dbf9f41c18be /dev/disk/by-path/ip-127.0.0.1:3260-iscsi-iqn.2016-06.world.srv:storage.target01-lun-1","size":10737418240,"uuid":"","serial":"36001405f826bd553d8c4dbf9f41c18be","type":"disk","rotational":true,"readOnly":false,"ownPartition":true,"filesystem":"","vendor":"LIO-ORG","model":"disk02","wwn":"0x6001405f826bd553","wwnVendorExtension":"0x6001405f826bd553d8c4dbf9f41c18be","empty":true}]` - cm := &v1.ConfigMap{ + cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "local-device-" + nodeName, Namespace: ns, @@ -101,16 +104,16 @@ func createDiscoverConfigmap(nodeName, ns string, clientset *fake.Clientset) err return err } -func createNode(nodeName string, condition v1.NodeConditionType, clientset *fake.Clientset) error { +func createNode(nodeName string, condition corev1.NodeConditionType, clientset *fake.Clientset) error { ctx := context.TODO() - node := &v1.Node{ + node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: nodeName, }, - Status: v1.NodeStatus{ - Conditions: []v1.NodeCondition{ + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ { - Type: condition, Status: v1.ConditionTrue, + Type: condition, Status: corev1.ConditionTrue, }, }, }, @@ -121,16 +124,26 @@ func createNode(nodeName string, condition v1.NodeConditionType, clientset *fake func TestAddRemoveNode(t *testing.T) { ctx := context.TODO() + namespace := "ns-add-remove" // create a storage spec with the given nodes/devices/dirs nodeName := "node23" + oldConditionExportFunc := updateConditionFunc + defer func() { + updateConditionFunc = oldConditionExportFunc + }() + // stub out the conditionExportFunc to do nothing. we do not have a fake Rook interface that + // allows us to interact with a CephCluster resource like the fake K8s clientset. + updateConditionFunc = func(c *clusterd.Context, namespaceName types.NamespacedName, conditionType cephv1.ConditionType, status corev1.ConditionStatus, reason cephv1.ClusterReasonType, message string) { + // do nothing + } + // set up a fake k8s client set and watcher to generate events that the operator will listen to clientset := fake.NewSimpleClientset() os.Setenv(k8sutil.PodNamespaceEnvVar, "rook-system") defer os.Unsetenv(k8sutil.PodNamespaceEnvVar) - nodeErr := createNode(nodeName, v1.NodeReady, clientset) - assert.Nil(t, nodeErr) + test.AddReadyNode(t, clientset, nodeName, "23.23.23.23") cmErr := createDiscoverConfigmap(nodeName, "rook-system", clientset) assert.Nil(t, cmErr) @@ -138,7 +151,7 @@ func TestAddRemoveNode(t *testing.T) { clientset.PrependWatchReactor("configmaps", k8stesting.DefaultWatchReactor(statusMapWatcher, nil)) clusterInfo := &cephclient.ClusterInfo{ - Namespace: "ns-add-remove", + Namespace: namespace, CephVersion: cephver.Nautilus, } clusterInfo.SetName("rook-ceph-test") @@ -180,21 +193,20 @@ func TestAddRemoveNode(t *testing.T) { startCompleted = true }() - // simulate the completion of the nodes orchestration mockNodeOrchestrationCompletion(c, nodeName, statusMapWatcher) - - // wait for orchestration to complete waitForOrchestrationCompletion(c, nodeName, &startCompleted) // verify orchestration for adding the node succeeded assert.True(t, startCompleted) - assert.Nil(t, startErr) + assert.NoError(t, startErr) + _, err := clientset.AppsV1().Deployments(namespace).Get(ctx, deploymentName(1), metav1.GetOptions{}) + assert.NoError(t, err) // simulate the OSD pod having been created osdPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ Name: "osdPod", Labels: map[string]string{k8sutil.AppAttr: AppName}}} - _, err := c.context.Clientset.CoreV1().Pods(c.clusterInfo.Namespace).Create(ctx, osdPod, metav1.CreateOptions{}) + _, err = c.context.Clientset.CoreV1().Pods(c.clusterInfo.Namespace).Create(ctx, osdPod, metav1.CreateOptions{}) assert.NoError(t, err) // mock the ceph calls that will be called during remove node @@ -206,7 +218,14 @@ func TestAddRemoveNode(t *testing.T) { } if args[0] == "osd" { if args[1] == "df" { - return `{"nodes":[{"id":0,"name":"osd.0","kb_used":0},{"id":1,"name":"osd.1","kb_used":0}]}`, nil + return `{"nodes":[{"id":1,"name":"osd.1","kb_used":0}]}`, nil + } + if args[1] == "dump" { + // OSD 1 is down and out + return `{"OSDs": [{"OSD": 1, "Up": 0, "In": 0}]}`, nil + } + if args[1] == "safe-to-destroy" { + return `{"safe_to_destroy":[1],"active":[],"missing_stats":[],"stored_pgs":[]}`, nil } if args[1] == "set" { return "", nil @@ -232,6 +251,9 @@ func TestAddRemoveNode(t *testing.T) { if args[1] == "find" { return `{"crush_location":{"host":"my-host"}}`, nil } + if args[1] == "ok-to-stop" { + return cephclientfake.OsdOkToStopOutput(1, []int{1}, true), nil + } } if args[0] == "df" && args[1] == "detail" { return `{"stats":{"total_bytes":0,"total_used_bytes":0,"total_avail_bytes":3072}}`, nil @@ -255,7 +277,6 @@ func TestAddRemoveNode(t *testing.T) { statusMapWatcher = watch.NewFake() clientset.PrependWatchReactor("configmaps", k8stesting.DefaultWatchReactor(statusMapWatcher, nil)) - // kick off the start of the removal orchestration in a goroutine startErr = nil startCompleted = false go func() { @@ -263,15 +284,22 @@ func TestAddRemoveNode(t *testing.T) { startCompleted = true }() - // simulate the completion of the nodes orchestration mockNodeOrchestrationCompletion(c, nodeName, statusMapWatcher) - - // wait for orchestration to complete waitForOrchestrationCompletion(c, nodeName, &startCompleted) // verify orchestration for removing the node succeeded assert.True(t, startCompleted) - assert.Nil(t, startErr) + assert.NoError(t, startErr) + // deployment should still exist; OSDs are removed by health monitor code only if they are down, + // out, and the user has set removeOSDsIfOutAndSafeToRemove + _, err = clientset.AppsV1().Deployments(namespace).Get(ctx, deploymentName(1), metav1.GetOptions{}) + assert.NoError(t, err) + + removeIfOutAndSafeToRemove := true + healthMon := NewOSDHealthMonitor(context, client.AdminClusterInfo(namespace), removeIfOutAndSafeToRemove, cephv1.CephClusterHealthCheckSpec{}) + healthMon.checkOSDHealth() + _, err = clientset.AppsV1().Deployments(namespace).Get(ctx, deploymentName(1), metav1.GetOptions{}) + assert.True(t, k8serrors.IsNotFound(err)) } func TestAddNodeFailure(t *testing.T) { @@ -283,7 +311,7 @@ func TestAddNodeFailure(t *testing.T) { clientset.PrependReactor("create", "jobs", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, errors.New("mock failed to create jobs") }) - nodeErr := createNode(nodeName, v1.NodeReady, clientset) + nodeErr := createNode(nodeName, corev1.NodeReady, clientset) assert.Nil(t, nodeErr) os.Setenv(k8sutil.PodNamespaceEnvVar, "rook-system") @@ -333,7 +361,9 @@ func TestAddNodeFailure(t *testing.T) { func TestGetPVCHostName(t *testing.T) { ctx := context.TODO() clientset := fake.NewSimpleClientset() - clusterInfo := &client.ClusterInfo{Namespace: "ns"} + clusterInfo := &cephclient.ClusterInfo{Namespace: "ns"} + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) c := &Cluster{context: &clusterd.Context{Clientset: clientset}, clusterInfo: clusterInfo} osdInfo := OSDInfo{ID: 23} pvcName := "test-pvc" @@ -352,7 +382,7 @@ func TestGetPVCHostName(t *testing.T) { }, } k8sutil.AddLabelToDeployment(OSDOverPVCLabelKey, pvcName, osdDeployment) - osdDeployment.Spec.Template.Spec.NodeSelector = map[string]string{v1.LabelHostname: "testnode"} + osdDeployment.Spec.Template.Spec.NodeSelector = map[string]string{corev1.LabelHostname: "testnode"} _, err = clientset.AppsV1().Deployments(c.clusterInfo.Namespace).Create(ctx, osdDeployment, metav1.CreateOptions{}) assert.NoError(t, err) @@ -365,7 +395,7 @@ func TestGetPVCHostName(t *testing.T) { // delete the deployment and get the host name based on the pod err = clientset.AppsV1().Deployments(c.clusterInfo.Namespace).Delete(ctx, osdDeployment.Name, metav1.DeleteOptions{}) assert.NoError(t, err) - osdPod := &v1.Pod{ + osdPod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "osd-23", Namespace: c.clusterInfo.Namespace, @@ -394,38 +424,69 @@ func TestGetOSDInfo(t *testing.T) { location := "root=default host=myhost zone=myzone" osd1 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "dev/logical-volume-path", CVMode: "raw", Location: location} osd2 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "vg1/lv1", CVMode: "lvm", LVBackedPV: true} - osd3 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: ""} + osd3 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "", CVMode: "raw"} osdProp := osdProperties{ crushHostname: node, - pvc: v1.PersistentVolumeClaimVolumeSource{ClaimName: "pvc"}, + pvc: corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pvc"}, selection: rookv1.Selection{}, - resources: v1.ResourceRequirements{}, + resources: corev1.ResourceRequirements{}, storeConfig: config.StoreConfig{}, } dataPathMap := &provisionConfig{ DataPathMap: opconfig.NewDatalessDaemonDataPathMap(c.clusterInfo.Namespace, c.spec.DataDirHostPath), } - d1, _ := c.makeDeployment(osdProp, osd1, dataPathMap) - osds1, _ := c.getOSDInfo(d1) - assert.Equal(t, 1, len(osds1)) - assert.Equal(t, osd1.ID, osds1[0].ID) - assert.Equal(t, osd1.BlockPath, osds1[0].BlockPath) - assert.Equal(t, osd1.CVMode, osds1[0].CVMode) - assert.Equal(t, location, osds1[0].Location) + t.Run("get info from PVC-based OSDs", func(t *testing.T) { + d1, _ := c.makeDeployment(osdProp, osd1, dataPathMap) + osdInfo1, _ := c.getOSDInfo(d1) + assert.Equal(t, osd1.ID, osdInfo1.ID) + assert.Equal(t, osd1.BlockPath, osdInfo1.BlockPath) + assert.Equal(t, osd1.CVMode, osdInfo1.CVMode) + assert.Equal(t, location, osdInfo1.Location) - d2, _ := c.makeDeployment(osdProp, osd2, dataPathMap) - osds2, _ := c.getOSDInfo(d2) - assert.Equal(t, 1, len(osds2)) - assert.Equal(t, osd2.ID, osds2[0].ID) - assert.Equal(t, osd2.BlockPath, osds2[0].BlockPath) - assert.Equal(t, osd2.CVMode, osds2[0].CVMode) - assert.Equal(t, osd2.LVBackedPV, osds2[0].LVBackedPV) + d2, _ := c.makeDeployment(osdProp, osd2, dataPathMap) + osdInfo2, _ := c.getOSDInfo(d2) + assert.Equal(t, osd2.ID, osdInfo2.ID) + assert.Equal(t, osd2.BlockPath, osdInfo2.BlockPath) + assert.Equal(t, osd2.CVMode, osdInfo2.CVMode) + assert.Equal(t, osd2.LVBackedPV, osdInfo2.LVBackedPV) - d3, _ := c.makeDeployment(osdProp, osd3, dataPathMap) - osds3, err := c.getOSDInfo(d3) - assert.Equal(t, 0, len(osds3)) - assert.NotNil(t, err) + // make deployment fails if block path is empty. allow it to create a valid deployment, then + // 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: ""}) + _, err = c.getOSDInfo(d3) + assert.Error(t, err) + }) + + t.Run("get info from node-based OSDs", func(t *testing.T) { + useAllDevices := true + osd4 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "", CVMode: "lvm", Location: location} + osd5 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "vg1/lv1", CVMode: "lvm"} + osdProp = osdProperties{ + crushHostname: node, + devices: []rookv1.Device{}, + pvc: corev1.PersistentVolumeClaimVolumeSource{}, + selection: rookv1.Selection{ + UseAllDevices: &useAllDevices, + }, + resources: corev1.ResourceRequirements{}, + storeConfig: config.StoreConfig{}, + metadataDevice: "", + } + + d4, _ := c.makeDeployment(osdProp, osd4, dataPathMap) + osdInfo4, _ := c.getOSDInfo(d4) + assert.Equal(t, osd4.ID, osdInfo4.ID) + assert.Equal(t, location, osdInfo4.Location) + + d5, _ := c.makeDeployment(osdProp, osd5, dataPathMap) + osdInfo5, _ := c.getOSDInfo(d5) + assert.Equal(t, osd5.ID, osdInfo5.ID) + assert.Equal(t, osd5.CVMode, osdInfo5.CVMode) + }) } func TestOSDPlacement(t *testing.T) { @@ -435,14 +496,14 @@ func TestOSDPlacement(t *testing.T) { assert.Nil(t, result.NodeAffinity) // the osd daemon placement is specified - prop.placement = rookv1.Placement{NodeAffinity: &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ + prop.placement = rookv1.Placement{NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ { - MatchExpressions: []v1.NodeSelectorRequirement{ + MatchExpressions: []corev1.NodeSelectorRequirement{ { Key: "label1", - Operator: v1.NodeSelectorOpIn, + Operator: corev1.NodeSelectorOpIn, Values: []string{"bar", "baz"}, }, }, @@ -457,14 +518,14 @@ func TestOSDPlacement(t *testing.T) { assert.Equal(t, "label1", result.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions[0].Key) // The prepare placement is specified and takes precedence over the osd placement - prop.preparePlacement = &rookv1.Placement{NodeAffinity: &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ + prop.preparePlacement = &rookv1.Placement{NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ { - MatchExpressions: []v1.NodeSelectorRequirement{ + MatchExpressions: []corev1.NodeSelectorRequirement{ { Key: "label2", - Operator: v1.NodeSelectorOpIn, + Operator: corev1.NodeSelectorOpIn, Values: []string{"foo", "bar"}, }, }, @@ -539,36 +600,33 @@ func TestGetOSDInfoWithCustomRoot(t *testing.T) { location := "root=custom-root host=myhost zone=myzone" osd1 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "dev/logical-volume-path", CVMode: "raw", Location: location} osd2 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "vg1/lv1", CVMode: "lvm", LVBackedPV: true, Location: location} - osd3 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "", Location: location} + osd3 := OSDInfo{ID: 3, UUID: "osd-uuid", BlockPath: "", CVMode: "lvm", Location: location} osdProp := osdProperties{ crushHostname: node, - pvc: v1.PersistentVolumeClaimVolumeSource{ClaimName: "pvc"}, + pvc: corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pvc"}, selection: rookv1.Selection{}, - resources: v1.ResourceRequirements{}, + resources: corev1.ResourceRequirements{}, storeConfig: config.StoreConfig{}, } dataPathMap := &provisionConfig{ DataPathMap: opconfig.NewDatalessDaemonDataPathMap(c.clusterInfo.Namespace, c.spec.DataDirHostPath), } d1, _ := c.makeDeployment(osdProp, osd1, dataPathMap) - osds1, _ := c.getOSDInfo(d1) - assert.Equal(t, 1, len(osds1)) - assert.Equal(t, osd1.ID, osds1[0].ID) - assert.Equal(t, osd1.BlockPath, osds1[0].BlockPath) - assert.Equal(t, osd1.CVMode, osds1[0].CVMode) - assert.Equal(t, location, osds1[0].Location) + osdInfo1, _ := c.getOSDInfo(d1) + assert.Equal(t, osd1.ID, osdInfo1.ID) + assert.Equal(t, osd1.BlockPath, osdInfo1.BlockPath) + assert.Equal(t, osd1.CVMode, osdInfo1.CVMode) + assert.Equal(t, location, osdInfo1.Location) d2, _ := c.makeDeployment(osdProp, osd2, dataPathMap) - osds2, _ := c.getOSDInfo(d2) - assert.Equal(t, 1, len(osds2)) - assert.Equal(t, osd2.ID, osds2[0].ID) - assert.Equal(t, osd2.BlockPath, osds2[0].BlockPath) - assert.Equal(t, osd2.CVMode, osds2[0].CVMode) - assert.Equal(t, osd2.LVBackedPV, osds2[0].LVBackedPV) - assert.Equal(t, location, osds2[0].Location) + osdInfo2, _ := c.getOSDInfo(d2) + assert.Equal(t, osd2.ID, osdInfo2.ID) + assert.Equal(t, osd2.BlockPath, osdInfo2.BlockPath) + assert.Equal(t, osd2.CVMode, osdInfo2.CVMode) + assert.Equal(t, osd2.LVBackedPV, osdInfo2.LVBackedPV) + assert.Equal(t, location, osdInfo2.Location) d3, _ := c.makeDeployment(osdProp, osd3, dataPathMap) - osds3, err := c.getOSDInfo(d3) - assert.Equal(t, 0, len(osds3)) - assert.NotNil(t, err) + _, 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 e8cf7b56d..8157c3ef3 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -77,7 +77,7 @@ OSD_UUID=%s OSD_STORE_FLAG="%s" OSD_DATA_DIR=/var/lib/ceph/osd/ceph-"$OSD_ID" CV_MODE=%s -DEVICE=%s +DEVICE="$%s" METADATA_DEVICE="$%s" WAL_DEVICE="$%s" @@ -274,10 +274,14 @@ var defaultTuneSlowSettings = []string{ "--osd-delete-sleep=2", // Time in seconds to sleep before next removal transaction } +func deploymentName(osdID int) string { + return fmt.Sprintf(osdAppNameFmt, osdID) +} + func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionConfig *provisionConfig) (*apps.Deployment, error) { // If running on Octopus, we don't need to use the host PID namespace var hostPID = !c.clusterInfo.CephVersion.IsAtLeastOctopus() - deploymentName := fmt.Sprintf(osdAppNameFmt, osd.ID) + deploymentName := deploymentName(osd.ID) replicaCount := int32(1) volumeMounts := controller.CephVolumeMounts(provisionConfig.DataPathMap, false) configVolumeMounts := controller.RookVolumeMounts(provisionConfig.DataPathMap, false) @@ -292,11 +296,9 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC doBinaryCopyInit := true // copy tini and rook binaries in an init container? doActivateOSDInit := false // run an init container to activate the osd? - // If CVMode is empty, this likely means we upgraded Rook - // This property did not exist before so we need to initialize it // This property is used for both PVC and non-PVC use case if osd.CVMode == "" { - osd.CVMode = "lvm" + return nil, errors.Errorf("failed to generate deployment for OSD %d. required CVMode is not specified for this OSD", osd.ID) } dataDir := k8sutil.DataDir @@ -343,6 +345,8 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC Name: "rook-ceph-config"}, Key: "mon_host"}}}, {Name: "CEPH_ARGS", Value: "-m $(ROOK_CEPH_MON_HOST)"}, + blockPathEnvVariable(osd.BlockPath), + cvModeEnvVariable(osd.CVMode), }...) configEnvVars := append(c.getConfigEnvVars(osdProps, dataDir), []v1.EnvVar{ tiniEnvVar, @@ -455,16 +459,12 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC // Let's use the old bridge for these lvm based pvc osds volumeMounts = append(volumeMounts, getPvcOSDBridgeMount(osdProps.pvc.ClaimName)) envVars = append(envVars, pvcBackedOSDEnvVar("true")) - envVars = append(envVars, blockPathEnvVariable(osd.BlockPath)) - envVars = append(envVars, cvModeEnvVariable(osd.CVMode)) envVars = append(envVars, lvBackedPVEnvVar(strconv.FormatBool(osd.LVBackedPV))) } if osdProps.onPVC() && osd.CVMode == "raw" { volumeMounts = append(volumeMounts, getPvcOSDBridgeMountActivate(osdDataDirPath, osdProps.pvc.ClaimName)) envVars = append(envVars, pvcBackedOSDEnvVar("true")) - envVars = append(envVars, blockPathEnvVariable(osd.BlockPath)) - envVars = append(envVars, cvModeEnvVariable(osd.CVMode)) } // We cannot go un-privileged until we have a bindmount for logs and crash @@ -742,7 +742,12 @@ func (c *Cluster) getActivateOSDInitContainer(configDir, namespace, osdID string Name: activateOSDVolumeName, VolumeSource: source, } - envVars := osdActivateEnvVar() + envVars := append( + osdActivateEnvVar(), + blockPathEnvVariable(osdInfo.BlockPath), + metadataDeviceEnvVar(osdInfo.MetadataPath), + walDeviceEnvVar(osdInfo.WalPath), + ) osdStore := "--bluestore" // Build empty dir osd path to something like "/var/lib/ceph/osd/ceph-0" @@ -762,7 +767,7 @@ func (c *Cluster) getActivateOSDInitContainer(configDir, namespace, osdID string Command: []string{ "/bin/bash", "-c", - fmt.Sprintf(activateOSDCode, osdID, osdInfo.UUID, osdStore, osdInfo.CVMode, osdInfo.BlockPath, osdMetadataDeviceEnvVarName, osdWalDeviceEnvVarName), + fmt.Sprintf(activateOSDCode, osdID, osdInfo.UUID, osdStore, osdInfo.CVMode, blockPathVarName, osdMetadataDeviceEnvVarName, osdWalDeviceEnvVarName), }, Name: "activate", Image: c.spec.CephVersion.Image, diff --git a/pkg/operator/ceph/cluster/osd/spec_test.go b/pkg/operator/ceph/cluster/osd/spec_test.go index 016a89afd..3e8ef6ca8 100644 --- a/pkg/operator/ceph/cluster/osd/spec_test.go +++ b/pkg/operator/ceph/cluster/osd/spec_test.go @@ -23,7 +23,6 @@ import ( cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" rookv1 "github.com/rook/rook/pkg/apis/rook.io/v1" "github.com/rook/rook/pkg/clusterd" - "github.com/rook/rook/pkg/daemon/ceph/client" cephclient "github.com/rook/rook/pkg/daemon/ceph/client" "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" opconfig "github.com/rook/rook/pkg/operator/ceph/config" @@ -32,13 +31,14 @@ import ( "github.com/rook/rook/pkg/operator/k8sutil" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/client-go/kubernetes/fake" ) func TestPodContainer(t *testing.T) { - cluster := &Cluster{rookVersion: "23", clusterInfo: client.AdminClusterInfo("myosd")} + cluster := &Cluster{rookVersion: "23", clusterInfo: cephclient.AdminClusterInfo("myosd")} cluster.clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) osdProps := osdProperties{ crushHostname: "node", @@ -117,7 +117,8 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { return } osd := OSDInfo{ - ID: 0, + ID: 0, + CVMode: "raw", } osdProp := osdProperties{ @@ -565,7 +566,8 @@ func TestHostNetwork(t *testing.T) { n := c.spec.Storage.ResolveNode(storageSpec.Nodes[0].Name) osd := OSDInfo{ - ID: 0, + ID: 0, + CVMode: "raw", } osdProp := osdProperties{ @@ -664,3 +666,43 @@ func TestClusterGetPVCEncryptionInitContainerActivate(t *testing.T) { containers = c.getPVCEncryptionInitContainerActivate(mountPath, osdProperties) assert.Equal(t, 3, len(containers)) } + +// WARNING! modifies c.ValidStorage +func getDummyDeploymentOnPVC(clientset *fake.Clientset, c *Cluster, pvcName string, osdID int) *appsv1.Deployment { + osd := OSDInfo{ + ID: osdID, + UUID: "some-uuid", + BlockPath: "/some/path", + CVMode: "raw", + } + c.ValidStorage.VolumeSources = append(c.ValidStorage.VolumeSources, rookv1.VolumeSource{ + Name: pvcName, + PVCSources: map[string]v1.PersistentVolumeClaimVolumeSource{ + bluestorePVCData: {ClaimName: pvcName}, + }, + Portable: true, + }) + config := c.newProvisionConfig() + d, err := deploymentOnPVC(c, osd, pvcName, config) + if err != nil { + panic(err) + } + return d +} + +// WARNING! modifies c.ValidStorage +func getDummyDeploymentOnNode(clientset *fake.Clientset, c *Cluster, nodeName string, osdID int) *appsv1.Deployment { + osd := OSDInfo{ + ID: osdID, + UUID: "some-uuid", + BlockPath: "/dev/vda", + CVMode: "raw", + } + c.ValidStorage.Nodes = append(c.ValidStorage.Nodes, rookv1.Node{Name: nodeName}) + config := c.newProvisionConfig() + d, err := deploymentOnNode(c, osd, nodeName, config) + if err != nil { + panic(err) + } + return d +} diff --git a/pkg/operator/ceph/cluster/osd/status.go b/pkg/operator/ceph/cluster/osd/status.go index 628597698..9943c7d0b 100644 --- a/pkg/operator/ceph/cluster/osd/status.go +++ b/pkg/operator/ceph/cluster/osd/status.go @@ -23,14 +23,14 @@ import ( "fmt" "time" + "github.com/pkg/errors" "github.com/rook/rook/pkg/operator/ceph/config" "github.com/rook/rook/pkg/operator/ceph/controller" "github.com/rook/rook/pkg/operator/k8sutil" - "github.com/rook/rook/pkg/util" - v1 "k8s.io/api/core/v1" - kerrors "k8s.io/apimachinery/pkg/api/errors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/watch" + k8swatch "k8s.io/apimachinery/pkg/watch" ) const ( @@ -41,9 +41,6 @@ const ( // OrchestrationStatusCompleted denotes the OSD provisioning has completed. This does not imply // the provisioning completed successfully in whole or in part. OrchestrationStatusCompleted = "completed" - // OrchestrationStatusAlreadyExists denotes the OSD provisioning was not started because an OSD - // has already been provisioned and started. - OrchestrationStatusAlreadyExists = "alreadyExists" // OrchestrationStatusFailed denotes the OSD provisioning has failed. OrchestrationStatusFailed = "failed" @@ -51,17 +48,18 @@ const ( orchestrationStatusKey = "status" provisioningLabelKey = "provisioning" nodeLabelKey = "node" - completeProvisionTimeout = 20 ) var ( // time to wait before updating OSDs opportunistically while waiting for OSDs to finish provisioning osdOpportunisticUpdateDuration = 100 * time.Millisecond + + // a ticker that ticks every minute to check progress + minuteTickerDuration = time.Minute ) type provisionConfig struct { - errorMessages []string - DataPathMap *config.DataPathMap // location to store data in container + DataPathMap *config.DataPathMap // location to store data in OSD and OSD prepare containers } func (c *Cluster) newProvisionConfig() *provisionConfig { @@ -70,26 +68,58 @@ func (c *Cluster) newProvisionConfig() *provisionConfig { } } -func (c *provisionConfig) addError(message string, args ...interface{}) { +// The provisionErrors struct can get passed around to provisioning code which can add errors to its +// internal list of errors. The errors will be reported at the end of provisioning. +type provisionErrors struct { + errors []error +} + +func newProvisionErrors() *provisionErrors { + return &provisionErrors{ + errors: []error{}, + } +} + +func (e *provisionErrors) addError(message string, args ...interface{}) { logger.Errorf(message, args...) - c.errorMessages = append(c.errorMessages, fmt.Sprintf(message, args...)) + e.errors = append(e.errors, errors.Errorf(message, args...)) } -func (c *Cluster) updateOSDStatus(node string, status OrchestrationStatus) { - UpdateNodeStatus(c.kv, node, status) +func (e *provisionErrors) len() int { + return len(e.errors) } -func UpdateNodeStatus(kv *k8sutil.ConfigMapKVStore, node string, status OrchestrationStatus) { - labels := map[string]string{ +func (e *provisionErrors) asMessages() string { + o := "" + for _, err := range e.errors { + o = fmt.Sprintf("%s\n%v", o, err) + } + return o +} + +// return name of status ConfigMap +func (c *Cluster) updateOSDStatus(node string, status OrchestrationStatus) string { + return UpdateNodeStatus(c.kv, node, status) +} + +func statusConfigMapLabels(node string) map[string]string { + return map[string]string{ k8sutil.AppAttr: AppName, orchestrationStatusKey: provisioningLabelKey, nodeLabelKey: node, } +} + +// UpdateNodeStatus updates the status ConfigMap for the OSD on the given node. It returns the name +// the ConfigMap used. +func UpdateNodeStatus(kv *k8sutil.ConfigMapKVStore, node string, status OrchestrationStatus) string { + labels := statusConfigMapLabels(node) // update the status map with the given status now s, _ := json.Marshal(status) + cmName := statusConfigMapName(node) if err := kv.SetValueWithLabels( - k8sutil.TruncateNodeName(orchestrationStatusMapName, node), + cmName, orchestrationStatusKey, string(s), labels, @@ -97,10 +127,11 @@ func UpdateNodeStatus(kv *k8sutil.ConfigMapKVStore, node string, status Orchestr // log the error, but allow the orchestration to continue even if the status update failed logger.Errorf("failed to set node %q status to %q for osd orchestration. %s", node, status.Status, status.Message) } + return cmName } -func (c *Cluster) handleOrchestrationFailure(config *provisionConfig, nodeName, message string) { - config.addError(message) +func (c *Cluster) handleOrchestrationFailure(errors *provisionErrors, nodeName, message string, args ...interface{}) { + errors.addError(message, args...) status := OrchestrationStatus{Status: OrchestrationStatusFailed, Message: message} UpdateNodeStatus(c.kv, nodeName, status) } @@ -125,313 +156,215 @@ func parseOrchestrationStatus(data map[string]string) *OrchestrationStatus { return &status } -func (c *Cluster) completeProvision(config *provisionConfig) bool { - return c.completeOSDsForAllNodes(config, true, completeProvisionTimeout) +// return errors from this function when OSD provisioning should stop and the reconcile be restarted +func (c *Cluster) updateAndCreateOSDs( + createConfig *createConfig, + updateConfig *updateConfig, + errs *provisionErrors, // add errors here +) error { + // tick every mintue to check-in on housekeeping stuff and report overall progress + minuteTicker := time.NewTicker(minuteTickerDuration) + defer minuteTicker.Stop() + + var err error + + doLoop := true + for doLoop { + doLoop, err = c.updateAndCreateOSDsLoop(createConfig, updateConfig, minuteTicker, errs) + if err != nil { + if !doLoop { + return err + } + logger.Errorf("%v", err) + } + } + + return nil } -func (c *Cluster) checkNodesCompleted(selector string, config *provisionConfig, configOSDs bool) (int, *util.Set, bool, *v1.ConfigMapList, []*v1.ConfigMap, error) { - ctx := context.TODO() - opts := metav1.ListOptions{ - LabelSelector: selector, - Watch: false, - } - remainingNodes := util.NewSet() - deferredConfigMaps := []*v1.ConfigMap{} - - // check the status map to see if the node is already completed before we start watching - statuses, err := c.context.Clientset.CoreV1().ConfigMaps(c.clusterInfo.Namespace).List(ctx, opts) - if err != nil { - if !kerrors.IsNotFound(err) { - config.addError("failed to get config status. %v", err) - return 0, remainingNodes, false, statuses, deferredConfigMaps, err - } - // the status map doesn't exist yet, watching below is still an OK thing to do - } - - originalNodes := len(statuses.Items) - // check the nodes to see which ones are already completed - for _, configMap := range statuses.Items { - node, ok := configMap.Labels[nodeLabelKey] - if !ok { - logger.Warningf("missing node label on configmap %s", configMap.Name) - continue - } - var completed bool - localconfigMap := configMap - completed, deferredConfigMaps = c.handleStatusConfigMapStatus(node, config, &localconfigMap, deferredConfigMaps, configOSDs) - if !completed { - remainingNodes.Add(node) - } - } - if remainingNodes.Count() == 0 { - return originalNodes, remainingNodes, true, statuses, deferredConfigMaps, nil - } - return originalNodes, remainingNodes, false, statuses, deferredConfigMaps, nil -} - -func (c *Cluster) completeOSDsForAllNodes(config *provisionConfig, configOSDs bool, timeoutMinutes int) bool { - ctx := context.TODO() - selector := fmt.Sprintf("%s=%s,%s=%s", +func statusConfigMapSelector() string { + return fmt.Sprintf("%s=%s,%s=%s", k8sutil.AppAttr, AppName, orchestrationStatusKey, provisioningLabelKey, ) +} - // For OSDs that are definitely update operations, we want to be able to defer the update until - // after we have created new OSDs in order to speed up cluster scale-out. Allow us to defer - // updating by keeping a list of deferred configmaps and processing those after creates have - // been done. If OSD provisioning doesn't report "alreadyExists" status, then no configmaps will - // be deferred, and OSD create/update ops will be intermixed in whatever order is processed. - deferredConfigMaps := []*v1.ConfigMap{} - // handleDeferredConfigMaps NEEDS to happen before the outer function returns. - // The nested for loops make it hard to break out and handle the return value cleanly, - // so remember to call this before all return statements. - handleDeferredConfigMaps := func() { - logger.Debugf("handling all deferred OSD statuses") - for _, configMap := range deferredConfigMaps { - node, ok := configMap.Labels[nodeLabelKey] - if !ok { - logger.Infof("missing node label on deferred configmap %q", configMap.Name) - continue - } +func (c *Cluster) updateAndCreateOSDsLoop( + createConfig *createConfig, + updateConfig *updateConfig, + minuteTicker *time.Ticker, // pass in the minute ticker so that we always know when a minute passes + errs *provisionErrors, // add errors here +) (shouldRestart bool, err error) { + cmClient := c.context.Clientset.CoreV1().ConfigMaps(c.clusterInfo.Namespace) + ctx := context.TODO() + selector := statusConfigMapSelector() - c.handleDeferredStatusConfigMapStatus(node, config, configMap, configOSDs) - } + listOptions := metav1.ListOptions{ + LabelSelector: selector, + } + configMapList, err := cmClient.List(ctx, listOptions) + if err != nil { + return false, errors.Wrapf(err, "failed to list OSD provisioning status ConfigMaps") } - var originalNodes int - var remainingNodes *util.Set - var completed bool - var statuses *v1.ConfigMapList - var err error - originalNodes, remainingNodes, completed, statuses, deferredConfigMaps, err = c.checkNodesCompleted(selector, config, configOSDs) - if err == nil && completed { - handleDeferredConfigMaps() - return true + // Process the configmaps initially in case any are already in a processable state + for i := range configMapList.Items { + // reference index to prevent implicit memory aliasing error + c.createOSDsForStatusMap(&configMapList.Items[i], createConfig, errs) } - // Make a timer to help us opportunistically handle OSD updates while waiting on OSD prepare - // jobs to finish. The timer value created here doesn't practically matter for the code below. - opportunityTimer := time.NewTimer(osdOpportunisticUpdateDuration) + watchOptions := metav1.ListOptions{ + LabelSelector: selector, + Watch: true, + ResourceVersion: configMapList.ResourceVersion, + } + watcher, err := cmClient.Watch(ctx, watchOptions) + defer watcher.Stop() + if err != nil { + return false, errors.Wrapf(err, "failed to start watching OSD provisioning status ConfigMaps") + } - // Make a timer that helps us log every minute that goes by without making progress and will - // be used to determine if we have gone past our timeout without making progress. - timeoutTimer := time.NewTimer(time.Minute) - shouldResetTimeoutTimer := true // set true when timeout timer should be reset - currentTimeoutMinutes := 0 // track how many minutes have gone by without progress + // tick after a short time of waiting for new OSD provision status configmaps to change state + // in order to allow opportunistic deployment updates while we wait + updateTicker := time.NewTicker(osdOpportunisticUpdateDuration) + defer updateTicker.Stop() + watchErrMsg := "failed during watch of OSD provisioning status ConfigMaps" for { - // Check whether we need to cancel the orchestration - if err := controller.CheckForCancelledOrchestration(c.context); err != nil { - config.addError("%s", err.Error()) - return false + if updateConfig.doneUpdating() && createConfig.doneCreating() { + break // loop } - opts := metav1.ListOptions{ - LabelSelector: selector, - Watch: true, - // start watching for changes on the orchestration status map - ResourceVersion: statuses.ResourceVersion, + // reset the update ticker (and drain the channel if necessary) to make sure we always + // wait a little bit for an OSD prepare result before opportunistically updating deployments + updateTicker.Reset(osdOpportunisticUpdateDuration) + if len(updateTicker.C) > 0 { + <-updateTicker.C } - logger.Infof("%d/%d node(s) completed osd provisioning, resource version %v", (originalNodes - remainingNodes.Count()), originalNodes, opts.ResourceVersion) - w, err := c.context.Clientset.CoreV1().ConfigMaps(c.clusterInfo.Namespace).Watch(ctx, opts) - if err != nil { - logger.Warningf("failed to start watch on osd status, trying again. %v", err) - time.Sleep(5 * time.Second) - continue - } - defer w.Stop() - - ResultLoop: - for { - // Reset the opportunity timer every time we start this loop. If the timer fires before - // there is a ConfigMap update (new OSD created), we opportunistically handle an OSD - // update and then return to the top of this loop. If the ConfigMap watcher has a result - // before the timeout, a new OSD will be created. The opportunity timer might fire while - // creation is happening. When the OSD is done being created, we return to the top of - // this loop and reset the timeout here to make sure a previous timer firing doesn't - // take update precedence away from any ConfigMap updates (new OSD) that need handled. - if !opportunityTimer.Stop() { - // the timer already fired - if len(opportunityTimer.C) > 0 { - // the timer fired, but the result channel needs drained before Reset below - <-opportunityTimer.C - } + select { + case event, ok := <-watcher.ResultChan(): + if !ok { + logger.Infof("restarting watcher for OSD provisioning status ConfigMaps. the watcher closed the channel") + return true, nil } - opportunityTimer.Reset(osdOpportunisticUpdateDuration) - if shouldResetTimeoutTimer { - if !timeoutTimer.Stop() { - // the timer already fired - if len(timeoutTimer.C) > 0 { - // the timer fired, but the result channel needs drained before Reset below - <-timeoutTimer.C - } - } - timeoutTimer.Reset(time.Minute) + if !isAddOrModifyEvent(event.Type) { + // We don't want to process delete events when we delete configmaps after having + // processed them. We also don't want to process BOOKMARK or ERROR events. + logger.Debugf("not processing %s event for object %q", event.Type, eventObjectName(event)) + break // case } - shouldResetTimeoutTimer = false // don't reset the timer unless a case below asks for it - select { - case e, ok := <-w.ResultChan(): - if !ok { - logger.Infof("orchestration status config map result channel closed, will restart watch.") - w.Stop() - <-time.After(5 * time.Second) - var leftNodes int - var leftRemainingNodes *util.Set - var completed bool - leftNodes, leftRemainingNodes, completed, _, deferredConfigMaps, err = c.checkNodesCompleted(selector, config, configOSDs) - if err == nil { - if completed { - logger.Infof("additional %d/%d node(s) completed osd provisioning", leftNodes, originalNodes) - handleDeferredConfigMaps() - return true - } - remainingNodes = leftRemainingNodes - } else { - logger.Warningf("failed to list orchestration configmap, status: %v", err) - } - break ResultLoop - } - if e.Type == watch.Modified { - configMap, ok := e.Object.(*v1.ConfigMap) - if !ok { - logger.Errorf("expected type ConfigMap but found %T", configMap) - continue - } - node, ok := configMap.Labels[nodeLabelKey] - if !ok { - logger.Infof("missing node label on configmap %s", configMap.Name) - continue - } - if !remainingNodes.Contains(node) { - logger.Infof("skipping event from node %s status update since it is already completed", node) - continue - } - var completed bool - completed, deferredConfigMaps = c.handleStatusConfigMapStatus(node, config, configMap, deferredConfigMaps, configOSDs) - if completed { - remainingNodes.Remove(node) - if remainingNodes.Count() == 0 { - logger.Infof("%d/%d node(s) completed osd provisioning", originalNodes, originalNodes) - handleDeferredConfigMaps() - return true - } - } - // made progress on creating/updating OSDs; reset the timeout timer - shouldResetTimeoutTimer = true - } - - case <-opportunityTimer.C: - // We want to make a best-effort attempt to update configmaps while provisioning is - // ongoing. While there are deferred configmaps remaining, grab one and process it. - if len(deferredConfigMaps) > 0 { - configMap := deferredConfigMaps[0] - logger.Debugf("opportunistically handling deferred OSD status %q", configMap.Name) - - node, ok := configMap.Labels[nodeLabelKey] - if !ok { - logger.Infof("missing node label on deferred configmap %q", configMap.Name) - continue - } - - c.handleDeferredStatusConfigMapStatus(node, config, configMap, configOSDs) - - // remove the configmap we just processed from the deferred configmap list - deferredConfigMaps = deferredConfigMaps[1:] - // made progress on updating OSDs; reset the timeout timer - shouldResetTimeoutTimer = true - } - - case <-timeoutTimer.C: - // Check whether we need to cancel the orchestration - if err := controller.CheckForCancelledOrchestration(c.context); err != nil { - config.addError("%s", err.Error()) - return false - } - // log every so often while we are waiting - currentTimeoutMinutes++ - if currentTimeoutMinutes == timeoutMinutes { - config.addError("timed out waiting for %d nodes: %+v", remainingNodes.Count(), remainingNodes) - // start to remove remainingNodes waiting timeout. - for remainingNode := range remainingNodes.Iter() { - clearNodeName := k8sutil.TruncateNodeName(orchestrationStatusMapName, remainingNode) - if err := c.kv.ClearStore(clearNodeName); err != nil { - config.addError("failed to clear node %q status with name %q. %v", remainingNode, clearNodeName, err) - } - } - handleDeferredConfigMaps() - return false - } - logger.Infof("waiting on orchestration status update from %d remaining nodes", remainingNodes.Count()) - // made no progress, but the timer needs reset so we can timeout again - shouldResetTimeoutTimer = true + configMap, ok := event.Object.(*corev1.ConfigMap) + if !ok { + logger.Errorf("recovering. %s. expected type ConfigMap but found %T", watchErrMsg, configMap) + break // case } + + c.createOSDsForStatusMap(configMap, createConfig, errs) + + case <-updateTicker.C: + // do an update + updateConfig.updateExistingOSDs(errs) + + case <-minuteTicker.C: + // Check whether we need to cancel the orchestration + if err := controller.CheckForCancelledOrchestration(c.context); err != nil { + return false, err + } + // Log progress + c, cExp := createConfig.progress() + u, uExp := updateConfig.progress() + logger.Infof("waiting... %d of %d OSD prepare jobs have finished processing and %d of %d OSDs have been updated", c, cExp, u, uExp) } } + + return false, nil +} + +func isAddOrModifyEvent(t k8swatch.EventType) bool { + switch t { + case k8swatch.Added, k8swatch.Modified: + return true + default: + return false + } } -func (c *Cluster) createOrUpdateOSDs(status *OrchestrationStatus, nodeName string, config *provisionConfig, configMap *v1.ConfigMap) { - if status.PvcBackedOSD { - c.startOSDDaemonsOnPVC(nodeName, config, configMap, status) - } else { - c.startOSDDaemonsOnNode(nodeName, config, configMap, status) - } - // remove the status configmap that indicated the progress - if err := c.kv.ClearStore(k8sutil.TruncateNodeName(orchestrationStatusMapName, nodeName)); err != nil { - logger.Errorf("failed to remove the status configmap %q. %v", orchestrationStatusMapName, err) +func eventObjectName(e k8swatch.Event) string { + objName := "[could not determine name]" + objMeta, _ := meta.Accessor(e.Object) + if objMeta != nil { + objName = objMeta.GetName() } + return objName } -func (c *Cluster) handleStatusConfigMapStatus(nodeName string, config *provisionConfig, configMap *v1.ConfigMap, deferredConfigMaps []*v1.ConfigMap, configOSDs bool) (completed bool, updatedDeferredConfigMaps []*v1.ConfigMap) { - // by default, do not append the input configmap to output to be deferred for processing later - updatedDeferredConfigMaps = deferredConfigMaps - - status := parseOrchestrationStatus(configMap.Data) - if status == nil { - return false, deferredConfigMaps - } - - logger.Infof("osd orchestration status for node %s is %s", nodeName, status.Status) - - if status.Status == OrchestrationStatusAlreadyExists { - logger.Debugf("deferring OSD update for node %q", nodeName) - - // in case where the OSD has already been provisioned, this is an update case which we want - // to defer until later - updatedDeferredConfigMaps = append(deferredConfigMaps, configMap) - - // we report that the node finished provisioning here but still rely on the caller to make - // sure to process deferred configmaps at a later point - return true, updatedDeferredConfigMaps - } - - if status.Status == OrchestrationStatusCompleted { - if configOSDs { - c.createOrUpdateOSDs(status, nodeName, config, configMap) - } - - return true, updatedDeferredConfigMaps - } - - if status.Status == OrchestrationStatusFailed { - config.addError("orchestration for node %s failed: %+v", nodeName, status) - return true, updatedDeferredConfigMaps - } - return false, updatedDeferredConfigMaps -} - -// deferred configmaps are OSD update operations we want to defer until after new OSDs have been created -func (c *Cluster) handleDeferredStatusConfigMapStatus(nodeName string, config *provisionConfig, configMap *v1.ConfigMap, configOSDs bool) { - status := parseOrchestrationStatus(configMap.Data) - if status == nil { - logger.Warningf("failed to handle deferred update of OSD for node %s. osd orchestration status for node %s is %s", nodeName, nodeName, status.Status) +// Create OSD Deployments for OSDs reported by the prepare job status configmap. +// Do not create OSD deployments if a deployment already exists for a given OSD. +func (c *Cluster) createOSDsForStatusMap( + configMap *corev1.ConfigMap, + createConfig *createConfig, + errs *provisionErrors, // add errors here +) { + nodeOrPVCName, ok := configMap.Labels[nodeLabelKey] + if !ok { + logger.Warningf("missing node label on configmap %s", configMap.Name) return } - if configOSDs { - logger.Infof("handling deferred update of OSD for node %q", nodeName) - c.createOrUpdateOSDs(status, nodeName, config, configMap) + status := parseOrchestrationStatus(configMap.Data) + if status == nil { + return + } + nodeOrPVC := "node" + if status.PvcBackedOSD { + nodeOrPVC = "PVC" + } + + logger.Infof("OSD orchestration status for %s %s is %q", nodeOrPVC, nodeOrPVCName, status.Status) + + if status.Status == OrchestrationStatusCompleted { + createConfig.createNewOSDsFromStatus(status, nodeOrPVCName, errs) + c.deleteStatusConfigMap(nodeOrPVCName) // remove the provisioning status configmap + return + } + + if status.Status == OrchestrationStatusFailed { + createConfig.doneWithStatus(nodeOrPVCName) + errs.addError("failed to provision OSD(s) on %s %s. %+v", nodeOrPVC, nodeOrPVCName, status) + c.deleteStatusConfigMap(nodeOrPVCName) // remove the provisioning status configmap + return + } +} + +func statusConfigMapName(nodeOrPVCName string) string { + return k8sutil.TruncateNodeName(orchestrationStatusMapName, nodeOrPVCName) +} + +func (c *Cluster) deleteStatusConfigMap(nodeOrPVCName string) { + if err := c.kv.ClearStore(statusConfigMapName(nodeOrPVCName)); err != nil { + logger.Errorf("failed to remove the status configmap %q. %v", statusConfigMapName(nodeOrPVCName), err) + } +} + +func (c *Cluster) deleteAllStatusConfigMaps() { + ctx := context.TODO() + listOpts := metav1.ListOptions{ + LabelSelector: statusConfigMapSelector(), + } + cmClientset := c.context.Clientset.CoreV1().ConfigMaps(c.clusterInfo.Namespace) + cms, err := cmClientset.List(ctx, listOpts) + if err != nil { + logger.Warningf("failed to clean up any dangling OSD prepare status configmaps. failed to list OSD prepare status configmaps. %v", err) + return + } + for _, cm := range cms.Items { + logger.Debugf("cleaning up dangling OSD prepare status configmap %q", cm.Name) + err := cmClientset.Delete(ctx, cm.Name, metav1.DeleteOptions{}) + if err != nil { + logger.Warningf("failed to clean up dangling OSD prepare status configmap %q. %v", cm.Name, err) + } } } diff --git a/pkg/operator/ceph/cluster/osd/status_test.go b/pkg/operator/ceph/cluster/osd/status_test.go index 9a0320855..3db934e91 100644 --- a/pkg/operator/ceph/cluster/osd/status_test.go +++ b/pkg/operator/ceph/cluster/osd/status_test.go @@ -75,7 +75,7 @@ func mockNodeOrchestrationCompletion(c *Cluster, nodeName string, statusMapWatch } for { // wait for the node's orchestration status to change to "starting" - cmName := fmt.Sprintf(orchestrationStatusMapName, nodeName) + cmName := statusConfigMapName(nodeName) cm, err := c.context.Clientset.CoreV1().ConfigMaps(c.clusterInfo.Namespace).Get(ctx, cmName, metav1.GetOptions{}) if err == nil { status := parseOrchestrationStatus(cm.Data) @@ -85,8 +85,11 @@ func mockNodeOrchestrationCompletion(c *Cluster, nodeName string, statusMapWatch status = &OrchestrationStatus{ OSDs: []OSDInfo{ { - ID: 1, - Cluster: "rook", + ID: 1, + UUID: "000000-0000-00000001", + Cluster: "rook", + CVMode: "raw", + BlockPath: "/dev/some/path", }, }, Status: OrchestrationStatusCompleted, @@ -114,7 +117,8 @@ func waitForOrchestrationCompletion(c *Cluster, nodeName string, startCompleted if *startCompleted { break } - cm, err := c.context.Clientset.CoreV1().ConfigMaps(c.clusterInfo.Namespace).Get(ctx, orchestrationStatusMapName, metav1.GetOptions{}) + cmName := statusConfigMapName(nodeName) + cm, err := c.context.Clientset.CoreV1().ConfigMaps(c.clusterInfo.Namespace).Get(ctx, cmName, metav1.GetOptions{}) if err == nil { status := parseOrchestrationStatus(cm.Data) if status != nil { diff --git a/pkg/operator/ceph/cluster/osd/update.go b/pkg/operator/ceph/cluster/osd/update.go new file mode 100644 index 000000000..b514bf943 --- /dev/null +++ b/pkg/operator/ceph/cluster/osd/update.go @@ -0,0 +1,339 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package osd + +import ( + "context" + "fmt" + "strconv" + "strings" + + "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + "github.com/rook/rook/pkg/operator/k8sutil" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// THE LIBRARY PROVIDED BY THIS FILE IS NOT THREAD SAFE + +var ( + // allow unit tests to override these values + maxUpdatesInParallel = 20 + updateMultipleDeploymentsAndWaitFunc = k8sutil.UpdateMultipleDeploymentsAndWait + deploymentOnNodeFunc = deploymentOnNode + deploymentOnPVCFunc = deploymentOnPVC + shouldCheckOkToStopFunc = cephclient.OSDUpdateShouldCheckOkToStop +) + +type updateConfig struct { + cluster *Cluster + provisionConfig *provisionConfig + queue *updateQueue // these OSDs need updated + numUpdatesNeeded int // the number of OSDs that needed updating + deployments *existenceList // these OSDs have existing deployments +} + +func (c *Cluster) newUpdateConfig( + provisionConfig *provisionConfig, + queue *updateQueue, + deployments *existenceList, +) *updateConfig { + return &updateConfig{ + c, + provisionConfig, + queue, + queue.Len(), + deployments, + } +} + +func (c *updateConfig) progress() (completed, initial int) { + return (c.numUpdatesNeeded - c.queue.Len()), c.numUpdatesNeeded +} + +func (c *updateConfig) doneUpdating() bool { + return c.queue.Len() == 0 +} + +func (c *updateConfig) updateExistingOSDs(errs *provisionErrors) { + ctx := context.TODO() + + if c.doneUpdating() { + return // no more OSDs to update + } + osdIDQuery, _ := c.queue.Pop() + + var osdIDs []int + var err error + if !shouldCheckOkToStopFunc(c.cluster.context, c.cluster.clusterInfo) { + // If we should not check ok-to-stop, then only process one OSD at a time. There are likely + // less than 3 OSDs in the cluster or the cluster is on a single node. E.g., in CI :wink:. + osdIDs = []int{osdIDQuery} + } else { + osdIDs, err = cephclient.OSDOkToStop(c.cluster.context, c.cluster.clusterInfo, osdIDQuery, maxUpdatesInParallel) + if err != nil { + if c.cluster.spec.ContinueUpgradeAfterChecksEvenIfNotHealthy { + logger.Infof("OSD %d is not ok-to-stop but 'continueUpgradeAfterChecksEvenIfNotHealthy' is true, so continuing to update it", osdIDQuery) + osdIDs = []int{osdIDQuery} // make sure to update the queried OSD + } else { + logger.Infof("OSD %d is not ok-to-stop. will try updating it again later", osdIDQuery) + c.queue.Push(osdIDQuery) // push back onto queue to make sure we retry it later + return + } + } + } + + logger.Debugf("updating OSDs: %v", osdIDs) + + updatedDeployments := make([]*appsv1.Deployment, 0, len(osdIDs)) + listIDs := []string{} // use this to build the k8s api selector query + for _, osdID := range osdIDs { + if !c.deployments.Exists(osdID) { + logger.Debugf("not updating deployment for OSD %d that is newly created", osdID) + continue + } + + // osdIDQuery which has been popped off the queue but it does need to be updated + if osdID != osdIDQuery && !c.queue.Exists(osdID) { + logger.Debugf("not updating deployment for OSD %d that is not in the update queue. the OSD has already been updated", osdID) + continue + } + + depName := deploymentName(osdID) + dep, err := c.cluster.context.Clientset.AppsV1().Deployments(c.cluster.clusterInfo.Namespace).Get(ctx, depName, metav1.GetOptions{}) + if err != nil { + errs.addError("failed to update OSD %d. failed to find existing deployment %q. %v", osdID, depName, err) + continue + } + osdInfo, err := c.cluster.getOSDInfo(dep) + if err != nil { + errs.addError("failed to update OSD %d. failed to extract OSD info from existing deployment %q. %v", osdID, depName, err) + continue + } + + nodeOrPVCName, err := getNodeOrPVCName(dep) + if err != nil { + errs.addError("%v", errors.Wrapf(err, "failed to update OSD %d", osdID)) + continue + } + + var updatedDep *appsv1.Deployment + if osdIsOnPVC(dep) { + logger.Infof("updating OSD %d on PVC %q", osdID, nodeOrPVCName) + updatedDep, err = deploymentOnPVCFunc(c.cluster, osdInfo, nodeOrPVCName, c.provisionConfig) + + message := fmt.Sprintf("Processing OSD %d on PVC %q", osdID, nodeOrPVCName) + updateConditionFunc(c.cluster.context, c.cluster.clusterInfo.NamespacedName(), cephv1.ConditionProgressing, v1.ConditionTrue, cephv1.ClusterProgressingReason, message) + } else { + if !c.cluster.ValidStorage.NodeExists(nodeOrPVCName) { + // node will not reconcile, so don't update the deployment + // allow the OSD health checker to remove the OSD + logger.Warningf( + "not updating OSD %d on node %q. node no longer exists in the storage spec. "+ + "if the user wishes to remove OSDs from the node, they must do so manually. "+ + "Rook will not remove OSDs from nodes that are removed from the storage spec in order to prevent accidental data loss", + osdID, nodeOrPVCName) + continue + } + + logger.Infof("updating OSD %d on node %q", osdID, nodeOrPVCName) + updatedDep, err = deploymentOnNodeFunc(c.cluster, osdInfo, nodeOrPVCName, c.provisionConfig) + + message := fmt.Sprintf("Processing OSD %d on node %q", osdID, nodeOrPVCName) + updateConditionFunc(c.cluster.context, c.cluster.clusterInfo.NamespacedName(), cephv1.ConditionProgressing, v1.ConditionTrue, cephv1.ClusterProgressingReason, message) + } + if err != nil { + errs.addError("%v", errors.Wrapf(err, "failed to update OSD %d", osdID)) + continue + } + + updatedDeployments = append(updatedDeployments, updatedDep) + listIDs = append(listIDs, strconv.Itoa(osdID)) + } + + // when waiting on deployments to be updated, only list OSDs we intend to update specifically by ID + listFunc := c.cluster.getFuncToListDeploymentsWithIDs(listIDs) + + failures := updateMultipleDeploymentsAndWaitFunc(c.cluster.context.Clientset, updatedDeployments, listFunc) + for _, f := range failures { + errs.addError("%v", errors.Wrapf(f.Error, "failed to update OSD deployment %q", f.ResourceName)) + } + + // If there were failures, don't retry them. If it's a transitory k8s/etcd issue, the next + // reconcile should succeed. If it's a different issue, it will always error. + c.queue.Remove(osdIDs) +} + +// getOSDUpdateInfo returns an update queue of OSDs which need updated and an existence list of OSD +// Deployments which already exist. +func (c *Cluster) getOSDUpdateInfo(errs *provisionErrors) (*updateQueue, *existenceList, error) { + ctx := context.TODO() + namespace := c.clusterInfo.Namespace + + selector := fmt.Sprintf("%s=%s", k8sutil.AppAttr, AppName) + listOpts := metav1.ListOptions{ + // list only rook-ceph-osd Deployments + LabelSelector: selector, + } + deps, err := c.context.Clientset.AppsV1().Deployments(namespace).List(ctx, listOpts) + if err != nil { + return nil, nil, errors.Wrap(err, "failed to query existing OSD deployments to see if they need updated") + } + + updateQueue := newUpdateQueueWithCapacity(len(deps.Items)) + existenceList := newExistenceListWithCapacity(len(deps.Items)) + + for i := range deps.Items { + id, err := getOSDID(&deps.Items[i]) // avoid implicit memory aliasing by indexing + if err != nil { + // add a question to the user AFTER the error text to help them recover from user error + errs.addError("%v. did a user create their own deployment with label %q?", selector, err) + continue + } + + // all OSD deployments should be marked as existing + existenceList.Add(id) + updateQueue.Push(id) + } + + return updateQueue, existenceList, nil +} + +// An updateQueue keeps track of OSDs which need updated. +type updateQueue struct { + q []int // just a list of OSD IDs +} + +// Create a new updateQueue with capacity reserved. +func newUpdateQueueWithCapacity(cap int) *updateQueue { + return &updateQueue{ + q: make([]int, 0, cap), + } +} + +func newUpdateQueueWithIDs(ids ...int) *updateQueue { + return &updateQueue{ + q: ids, + } +} + +// Len returns the length of the queue. +func (q *updateQueue) Len() int { + return len(q.q) +} + +// Push pushes an item onto the end of the queue. +func (q *updateQueue) Push(osdID int) { + q.q = append(q.q, osdID) +} + +// Pop pops an item off the beginning of the queue. +// Returns -1 and ok=false if the queue is empty. Otherwise, returns an OSD ID and ok=true. +func (q *updateQueue) Pop() (osdID int, ok bool) { + if q.Len() == 0 { + return -1, false + } + + osdID = q.q[0] + q.q = q.q[1:] + return osdID, true +} + +// Exists returns true if the item exists in the queue. +func (q *updateQueue) Exists(osdID int) bool { + for _, id := range q.q { + if id == osdID { + return true + } + } + return false +} + +// Remove removes the items from the queue if they exist. +func (q *updateQueue) Remove(osdIDs []int) { + shouldRemove := func(rid int) bool { + for _, id := range osdIDs { + if id == rid { + return true + } + } + return false + } + + lastIdx := 0 + for idx, osdID := range q.q { + if !shouldRemove(osdID) { + // do removal by shifting slice items that should be kept into the next good position in + // the slice, and then reduce the slice capacity to match the number of kept items + q.q[lastIdx] = q.q[idx] + lastIdx++ + } + } + q.q = q.q[:lastIdx] +} + +// An existenceList keeps track of which OSDs already have Deployments created for them that is +// queryable in O(1) time. +type existenceList struct { + m map[int]bool +} + +// Create a new existenceList with capacity reserved. +func newExistenceListWithCapacity(cap int) *existenceList { + return &existenceList{ + m: make(map[int]bool, cap), + } +} + +func newExistenceListWithIDs(ids ...int) *existenceList { + e := newExistenceListWithCapacity(len(ids)) + for _, id := range ids { + e.Add(id) + } + return e +} + +// Len returns the length of the existence list, the number of existing items. +func (e *existenceList) Len() int { + return len(e.m) +} + +// Add adds an item to the existenceList. +func (e *existenceList) Add(osdID int) { + e.m[osdID] = true +} + +// Exists returns true if an item is recorded in the existence list or false if it does not. +func (e *existenceList) Exists(osdID int) bool { + _, ok := e.m[osdID] + return ok +} + +// return a function that will list only OSD deployments with the IDs given +func (c *Cluster) getFuncToListDeploymentsWithIDs(osdIDs []string) func() (*appsv1.DeploymentList, error) { + ctx := context.TODO() + selector := fmt.Sprintf("ceph-osd-id in (%s)", strings.Join(osdIDs, ", ")) + listOpts := metav1.ListOptions{ + LabelSelector: selector, // e.g. 'ceph-osd-id in (1, 3, 5, 7, 9)' + } + return func() (*appsv1.DeploymentList, error) { + return c.context.Clientset.AppsV1().Deployments(c.clusterInfo.Namespace).List(ctx, listOpts) + } +} diff --git a/pkg/operator/ceph/cluster/osd/update_test.go b/pkg/operator/ceph/cluster/osd/update_test.go new file mode 100644 index 000000000..289ca8798 --- /dev/null +++ b/pkg/operator/ceph/cluster/osd/update_test.go @@ -0,0 +1,713 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package osd + +import ( + "context" + "fmt" + "strconv" + "testing" + + "github.com/coreos/pkg/capnslog" + "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + "github.com/rook/rook/pkg/clusterd" + cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + cephclientfake "github.com/rook/rook/pkg/daemon/ceph/client/fake" + "github.com/rook/rook/pkg/operator/ceph/controller" + cephver "github.com/rook/rook/pkg/operator/ceph/version" + "github.com/rook/rook/pkg/operator/k8sutil" + "github.com/rook/rook/pkg/operator/test" + exectest "github.com/rook/rook/pkg/util/exec/test" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" +) + +func Test_updateExistingOSDs(t *testing.T) { + namespace := "my-namespace" + + logger.SetLevel(capnslog.DEBUG) + + oldUpdateFunc := updateMultipleDeploymentsAndWaitFunc + oldNodeFunc := deploymentOnNodeFunc + oldPVCFunc := deploymentOnPVCFunc + oldConditionFunc := updateConditionFunc + oldShouldCheckFunc := shouldCheckOkToStopFunc + defer func() { + updateMultipleDeploymentsAndWaitFunc = oldUpdateFunc + deploymentOnNodeFunc = oldNodeFunc + deploymentOnPVCFunc = oldPVCFunc + updateConditionFunc = oldConditionFunc + shouldCheckOkToStopFunc = oldShouldCheckFunc + }() + + var executor *exectest.MockExecutor // will be defined later + + // inputs + var ( + updateQueue *updateQueue + existingDeployments *existenceList + clientset *fake.Clientset + ) + + // behavior control + var ( + updateInjectFailures k8sutil.Failures // return failures from mocked updateDeploymentAndWaitFunc + returnOkToStopIDs []int // return these IDs are ok-to-stop (or not ok to stop if empty) + forceUpgradeIfUnhealthy bool + ) + + // intermediates (created from inputs) + var ( + ctx *clusterd.Context + c *Cluster + updateConfig *updateConfig + ) + + // outputs + var ( + osdToBeQueried int // this OSD ID should be queried + deploymentsUpdated []string // updateDeploymentAndWaitFunc adds deployments to this list + osdsOnPVCs []int // deploymentOnPVCFunc adds OSD IDs to this list + osdsOnNodes []int // deploymentOnPVCFunc adds OSD IDs to this list + errs *provisionErrors + ) + + doSetup := func() { + // set up intermediates + ctx = &clusterd.Context{ + Clientset: clientset, + Executor: executor, + } + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + CephVersion: cephver.Pacific, + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + spec := cephv1.ClusterSpec{ + ContinueUpgradeAfterChecksEvenIfNotHealthy: forceUpgradeIfUnhealthy, + } + c = New(ctx, clusterInfo, spec, "rook/rook:master") + config := c.newProvisionConfig() + updateConfig = c.newUpdateConfig(config, updateQueue, existingDeployments) + + // prepare outputs + deploymentsUpdated = []string{} + osdsOnPVCs = []int{} + osdsOnNodes = []int{} + errs = newProvisionErrors() + } + + // stub out the conditionExportFunc to do nothing. we do not have a fake Rook interface that + // allows us to interact with a CephCluster resource like the fake K8s clientset. + updateConditionFunc = func(c *clusterd.Context, namespaceName types.NamespacedName, conditionType cephv1.ConditionType, status corev1.ConditionStatus, reason cephv1.ClusterReasonType, message string) { + // do nothing + } + shouldCheckOkToStopFunc = func(context *clusterd.Context, clusterInfo *cephclient.ClusterInfo) bool { + // always check. if shouldCheckOkToStop is not implemented correctly, single-node CI tests + // will fail, which is a more thorough test than we could make in unit tests. + return true + } + + updateMultipleDeploymentsAndWaitFunc = + func( + clientset kubernetes.Interface, + deployments []*appsv1.Deployment, + listFunc func() (*appsv1.DeploymentList, error), + ) k8sutil.Failures { + for _, d := range deployments { + deploymentsUpdated = append(deploymentsUpdated, d.Name) + } + return updateInjectFailures + } + + executor = &exectest.MockExecutor{ + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { + t.Logf("command: %s %v", command, args) + if fmt.Sprintf("%s %s %s", command, args[0], args[1]) != "ceph osd ok-to-stop" { + panic(fmt.Sprintf("unexpected command %q with args %v", command, args)) + } + queriedID := args[2] + if strconv.Itoa(osdToBeQueried) != queriedID { + err := errors.Errorf("OSD %d should have been queried, but %s was queried instead", osdToBeQueried, queriedID) + t.Error(err) + return "", err + } + if len(returnOkToStopIDs) > 0 { + return cephclientfake.OsdOkToStopOutput(osdToBeQueried, returnOkToStopIDs, true), nil + } + return cephclientfake.OsdOkToStopOutput(osdToBeQueried, []int{}, true), errors.Errorf("induced error") + }, + } + + // simple wrappers to allow us to count how many OSDs on nodes/PVCs are identified + deploymentOnNodeFunc = func(c *Cluster, osd OSDInfo, nodeName string, config *provisionConfig) (*appsv1.Deployment, error) { + osdsOnNodes = append(osdsOnNodes, osd.ID) + return deploymentOnNode(c, osd, nodeName, config) + } + deploymentOnPVCFunc = func(c *Cluster, osd OSDInfo, pvcName string, config *provisionConfig) (*appsv1.Deployment, error) { + osdsOnPVCs = append(osdsOnPVCs, osd.ID) + return deploymentOnPVC(c, osd, pvcName, config) + } + + addDeploymentOnNode := func(nodeName string, osdID int) { + d := getDummyDeploymentOnNode(clientset, c, nodeName, osdID) + _, err := clientset.AppsV1().Deployments(namespace).Create(context.TODO(), d, metav1.CreateOptions{}) + if err != nil { + panic(err) + } + } + + addDeploymentOnPVC := func(pvcName string, osdID int) { + d := getDummyDeploymentOnPVC(clientset, c, pvcName, osdID) + _, err := clientset.AppsV1().Deployments(namespace).Create(context.TODO(), d, metav1.CreateOptions{}) + if err != nil { + panic(err) + } + } + + t.Run("no items in the update queue should be a noop", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs() + existingDeployments = newExistenceListWithIDs(0, 2, 4, 6) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + osdToBeQueried = -1 // this will make any OSD query fail + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{}) + }) + + t.Run("ok to stop one OSD at a time", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + // reminder that updateQueue pops things off the queue from the front, so the leftmost item + // will be the one queried + updateQueue = newUpdateQueueWithIDs(0, 2, 4, 6) + existingDeployments = newExistenceListWithIDs(0, 2, 4, 6) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + for _, i := range []int{0, 2, 4, 6} { + osdToBeQueried = i + returnOkToStopIDs = []int{i} + deploymentsUpdated = []string{} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(i)}) + } + assert.ElementsMatch(t, osdsOnNodes, []int{0, 4}) + assert.ElementsMatch(t, osdsOnPVCs, []int{2, 6}) + + assert.Equal(t, 0, updateQueue.Len()) // should be done with updates + }) + + t.Run("ok to stop 3 OSDs at a time", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs(0, 2, 4, 6) + existingDeployments = newExistenceListWithIDs(0, 2, 4, 6) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + osdToBeQueried = 0 + returnOkToStopIDs = []int{0, 4, 6} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, + []string{deploymentName(0), deploymentName(4), deploymentName(6)}) + + // should NOT be done with updates + // this also tests that updateQueue.Len() directly affects doneUpdating() + assert.Equal(t, 1, updateQueue.Len()) + assert.False(t, updateConfig.doneUpdating()) + + deploymentsUpdated = []string{} + osdToBeQueried = 2 + returnOkToStopIDs = []int{2} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, + []string{deploymentName(2)}) + + // should be done with updates + // this also tests that updateQueue.Len() directly affects doneUpdating() + assert.Equal(t, 0, updateQueue.Len()) + assert.True(t, updateConfig.doneUpdating()) + }) + + t.Run("ok to stop more OSDs than are in the update queue", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs(2, 0) + existingDeployments = newExistenceListWithIDs(6, 4, 2, 0) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + osdToBeQueried = 2 + returnOkToStopIDs = []int{2, 4, 6} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(2)}) + + deploymentsUpdated = []string{} + osdToBeQueried = 0 + returnOkToStopIDs = []int{0, 6} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(0)}) + + assert.Equal(t, 0, updateQueue.Len()) // should be done with updates + }) + + t.Run("ok to stop OSDS not in existence list (newly-created OSDs)", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs(2, 0) + existingDeployments = newExistenceListWithIDs(2, 0) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + osdToBeQueried = 2 + returnOkToStopIDs = []int{2, 4, 6} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(2)}) + + deploymentsUpdated = []string{} + osdToBeQueried = 0 + returnOkToStopIDs = []int{0, 6} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(0)}) + + assert.Equal(t, 0, updateQueue.Len()) // should be done with updates + }) + + t.Run("not ok to stop OSD", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs(2) + existingDeployments = newExistenceListWithIDs(0, 2, 4, 6) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + osdToBeQueried = 2 + returnOkToStopIDs = []int{} // not ok to stop + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{}) + assert.Equal(t, 1, updateQueue.Len()) // the OSD should have been requeued + + osdToBeQueried = 2 + returnOkToStopIDs = []int{2} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(2)}) + assert.Equal(t, 0, updateQueue.Len()) // the OSD should now have been removed from the queue + }) + + t.Run("continueUpgradesAfterChecksEvenIfUnhealthy = true", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs(2) + existingDeployments = newExistenceListWithIDs(0, 2, 4, 6) + forceUpgradeIfUnhealthy = true // FORCE UPDATES + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + osdToBeQueried = 2 + returnOkToStopIDs = []int{} // NOT ok-to-stop + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(2)}) + + assert.Equal(t, 0, updateQueue.Len()) // should be done with updates + }) + + t.Run("failures updating deployments", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs(0, 2, 4, 6) + existingDeployments = newExistenceListWithIDs(0, 2, 4, 6) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + + osdToBeQueried = 0 + returnOkToStopIDs = []int{0, 6} + updateInjectFailures = k8sutil.Failures{ + {ResourceName: deploymentName(6), Error: errors.Errorf("induced failure updating OSD 6")}, + } + updateConfig.updateExistingOSDs(errs) + assert.Equal(t, 1, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, + []string{deploymentName(0), deploymentName(6)}) + + deploymentsUpdated = []string{} + osdToBeQueried = 2 + returnOkToStopIDs = []int{2, 4} + updateInjectFailures = k8sutil.Failures{ + {ResourceName: deploymentName(2), Error: errors.Errorf("induced failure updating OSD 2")}, + {ResourceName: deploymentName(4), Error: errors.Errorf("induced failure waiting for OSD 4")}, + } + updateConfig.updateExistingOSDs(errs) + assert.Equal(t, 3, errs.len()) // errors should be appended to the same provisionErrors struct + assert.ElementsMatch(t, deploymentsUpdated, + []string{deploymentName(2), deploymentName(4)}) + + assert.Zero(t, updateQueue.Len()) // errors should not be requeued + }) + + t.Run("failure due to OSD deployment with bad info", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + updateQueue = newUpdateQueueWithIDs(0, 6) + existingDeployments = newExistenceListWithIDs(0, 2, 4, 6) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnPVC("pvc2", 2) + addDeploymentOnNode("node1", 4) + addDeploymentOnPVC("pvc6", 6) + // give OSD 6 bad info by removing env vars from primary container + deploymentClient := clientset.AppsV1().Deployments(namespace) + d, err := deploymentClient.Get(context.TODO(), deploymentName(6), metav1.GetOptions{}) + if err != nil { + panic(err) + } + d.Spec.Template.Spec.Containers[0].Env = []corev1.EnvVar{} + _, err = deploymentClient.Update(context.TODO(), d, metav1.UpdateOptions{}) + if err != nil { + panic(err) + } + deploymentsUpdated = []string{} + + osdToBeQueried = 0 + returnOkToStopIDs = []int{0, 6} + updateConfig.updateExistingOSDs(errs) + assert.Equal(t, 1, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, + []string{deploymentName(0)}) + + assert.Zero(t, updateQueue.Len()) // errors should not be requeued + }) + + t.Run("do not update OSDs on nodes removed from the storage spec", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + // reminder that updateQueue pops things off the queue from the front, so the leftmost item + // will be the one queried + updateQueue = newUpdateQueueWithIDs(0, 4) + existingDeployments = newExistenceListWithIDs(0, 4) + forceUpgradeIfUnhealthy = false + updateInjectFailures = k8sutil.Failures{} + doSetup() + addDeploymentOnNode("node0", 0) + addDeploymentOnNode("node1", 4) + + // Remove "node0" from valid storage (user no longer wants it) + assert.Equal(t, "node0", c.ValidStorage.Nodes[0].Name) + c.ValidStorage.Nodes = c.ValidStorage.Nodes[1:] + t.Logf("valid storage nodes: %+v", c.ValidStorage.Nodes) + + osdToBeQueried = 0 + returnOkToStopIDs = []int{0, 4} + updateConfig.updateExistingOSDs(errs) + assert.Zero(t, errs.len()) + assert.ElementsMatch(t, deploymentsUpdated, []string{deploymentName(4)}) + + assert.ElementsMatch(t, osdsOnNodes, []int{4}) + assert.ElementsMatch(t, osdsOnPVCs, []int{}) + + assert.Equal(t, 0, updateQueue.Len()) // should be done with updates + }) +} + +func Test_getOSDUpdateInfo(t *testing.T) { + namespace := "rook-ceph" + cephImage := "ceph/ceph:v15" + + // NOTE: all tests share the same clientset + clientset := fake.NewSimpleClientset() + ctx := &clusterd.Context{ + Clientset: clientset, + } + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + CephVersion: cephver.Nautilus, + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + spec := cephv1.ClusterSpec{ + CephVersion: cephv1.CephVersionSpec{Image: cephImage}, + } + c := New(ctx, clusterInfo, spec, "rook/rook:master") + + var errs *provisionErrors + var d *appsv1.Deployment + + t.Run("cluster with no existing deployments", func(t *testing.T) { + errs = newProvisionErrors() + updateQueue, existenceList, err := c.getOSDUpdateInfo(errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.Zero(t, updateQueue.Len()) + assert.Zero(t, existenceList.Len()) + }) + + t.Run("cluster in namespace with existing deployments, but none are OSDs", func(t *testing.T) { + // random deployment in this namespace + addTestDeployment(clientset, "non-rook-deployment", namespace, map[string]string{}) + + // mon.a in this namespace + l := controller.CephDaemonAppLabels("rook-ceph-mon", namespace, "mon", "a", true) + addTestDeployment(clientset, "rook-ceph-mon-a", namespace, l) + + // osd.1 and 3 in another namespace (another Rook cluster) + clusterInfo2 := &cephclient.ClusterInfo{ + Namespace: "other-namespace", + CephVersion: cephver.Nautilus, + } + clusterInfo2.SetName("other-cluster") + clusterInfo2.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + c2 := New(ctx, clusterInfo2, spec, "rook/rook:master") + + // osd.1 on PVC in "other-namespace" + d = getDummyDeploymentOnPVC(clientset, c2, "pvc1", 1) + createDeploymentOrPanic(clientset, d) + + // osd.3 on Node in "other-namespace" + d = getDummyDeploymentOnNode(clientset, c2, "node3", 3) + createDeploymentOrPanic(clientset, d) + + errs = newProvisionErrors() + updateQueue, existenceList, err := c.getOSDUpdateInfo(errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.Zero(t, updateQueue.Len()) + assert.Zero(t, existenceList.Len()) + }) + + t.Run("cluster in namespace with existing OSD deployments", func(t *testing.T) { + // osd.0 on PVC in this namespace + d = getDummyDeploymentOnPVC(clientset, c, "pvc0", 0) + createDeploymentOrPanic(clientset, d) + + // osd.2 on Node in this namespace + d = getDummyDeploymentOnNode(clientset, c, "node2", 2) + createDeploymentOrPanic(clientset, d) + + errs = newProvisionErrors() + updateQueue, existenceList, err := c.getOSDUpdateInfo(errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.Equal(t, 2, updateQueue.Len()) + assert.True(t, updateQueue.Exists(0)) + assert.True(t, updateQueue.Exists(2)) + assert.Equal(t, 2, existenceList.Len()) + assert.True(t, existenceList.Exists(0)) + assert.True(t, existenceList.Exists(2)) + }) + + t.Run("existing OSD deployment with no OSD ID", func(t *testing.T) { + l := map[string]string{k8sutil.AppAttr: AppName} + addTestDeployment(clientset, "rook-ceph-osd-NOID", namespace, l) + + errs = newProvisionErrors() + updateQueue, existenceList, err := c.getOSDUpdateInfo(errs) + assert.NoError(t, err) + assert.Equal(t, 1, errs.len()) + // should have same update queue and existence list as last test + assert.Equal(t, 2, updateQueue.Len()) + assert.Equal(t, 2, existenceList.Len()) + }) + + t.Run("failure to list OSD deployments", func(t *testing.T) { + // reset the test to check that an error is reported if listing OSD deployments fails + test.PrependFailReactor(t, clientset, "list", "deployments") + ctx = &clusterd.Context{ + Clientset: clientset, + } + c = New(ctx, clusterInfo, spec, "rook/rook:master") + + errs = newProvisionErrors() + _, _, err := c.getOSDUpdateInfo(errs) + fmt.Println(err) + assert.Error(t, err) + }) +} + +func addTestDeployment(clientset *fake.Clientset, name, namespace string, labels map[string]string) { + d := &appsv1.Deployment{} + d.SetName(name) + d.SetNamespace(namespace) + d.SetLabels(labels) + createDeploymentOrPanic(clientset, d) +} + +func createDeploymentOrPanic(clientset *fake.Clientset, d *appsv1.Deployment) { + _, err := clientset.AppsV1().Deployments(d.Namespace).Create(context.TODO(), d, metav1.CreateOptions{}) + if err != nil { + panic(err) + } +} + +func Test_updateQueue(t *testing.T) { + q := newUpdateQueueWithCapacity(2) + assert.Equal(t, 2, cap(q.q)) + assert.Zero(t, q.Len()) + + testPop := func(osdID int) { + t.Helper() + id, ok := q.Pop() + assert.Equal(t, osdID, id) + assert.True(t, ok) + } + + assertEmpty := func() { + t.Helper() + id, ok := q.Pop() + assert.Equal(t, -1, id) + assert.False(t, ok) + } + + // assert empty behavior initially + assertEmpty() + + // test basic functionality + q.Push(0) + assert.Equal(t, 1, q.Len()) + testPop(0) + assertEmpty() + + // test that queue can hold more items than initial capacity + // and that items are Pushed/Popped in FIFO order + q.Push(1) + q.Push(2) + q.Push(3) + assert.Equal(t, 3, q.Len()) + assert.True(t, q.Exists(1)) + assert.True(t, q.Exists(2)) + assert.True(t, q.Exists(3)) + testPop(1) + testPop(2) + testPop(3) + assertEmpty() + + // Test removing queue items via q.Remove + for _, i := range []int{4, 5, 6, 7, 8} { + q.Push(i) + assert.True(t, q.Exists(i)) + } + assert.Equal(t, 5, q.Len()) + q.Remove([]int{ + 1, 2, 3, // non-existent items shouldn't affect queue + 4, // remove first item + 6, // remove a middle item + 8, // remove last item + }) + assert.Equal(t, 2, q.Len()) + assert.False(t, q.Exists(1)) + assert.False(t, q.Exists(2)) + assert.False(t, q.Exists(3)) + assert.True(t, q.Exists(5)) + assert.False(t, q.Exists(6)) + assert.True(t, q.Exists(7)) + assert.False(t, q.Exists(8)) + testPop(5) + // items pushed back onto the queue after removal should not get old values + q.Push(9) + q.Push(10) + assert.Equal(t, 3, q.Len()) + assert.False(t, q.Exists(5)) + assert.True(t, q.Exists(7)) + assert.True(t, q.Exists(9)) + assert.True(t, q.Exists(10)) + testPop(7) + testPop(9) + testPop(10) + assertEmpty() +} + +func Test_existenceList(t *testing.T) { + l := newExistenceListWithCapacity(2) + + // Assert zero item does not exist initially + assert.False(t, l.Exists(0)) + assert.Zero(t, l.Len()) + + // Assert basic functionality + l.Add(1) + assert.True(t, l.Exists(1)) + assert.False(t, l.Exists(0)) + assert.False(t, l.Exists(2)) + assert.Equal(t, 1, l.Len()) + + // assert that more items can be added than initial capacity + l.Add(0) + l.Add(2) + l.Add(3) + assert.True(t, l.Exists(0)) + assert.True(t, l.Exists(1)) // 1 should still exist from before + assert.True(t, l.Exists(2)) + assert.True(t, l.Exists(3)) + assert.False(t, l.Exists(4)) + assert.Equal(t, 4, l.Len()) + + // assert that the same item can be added twice (though this should never happen for OSDs IRL) + l.Add(1) + assert.True(t, l.Exists(1)) + assert.Equal(t, 4, l.Len()) +} diff --git a/pkg/operator/ceph/cluster/predicate.go b/pkg/operator/ceph/cluster/predicate.go index 3653f8512..a42ea69c5 100644 --- a/pkg/operator/ceph/cluster/predicate.go +++ b/pkg/operator/ceph/cluster/predicate.go @@ -132,23 +132,19 @@ func watchControllerPredicate(rookContext *clusterd.Context) predicate.Funcs { } diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer) if diff != "" { + // Set the cancellation flag to stop any ongoing orchestration + rookContext.RequestCancelOrchestration.Set() + logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff) - - // If spec Images are different we cancel any on-going orchestration - if objOld.Spec.CephVersion.Image != objNew.Spec.CephVersion.Image { - // Set the cancellation flag to stop any ongoing orchestration - rookContext.RequestCancelOrchestration.Set() - - logger.Info("upgrade requested, cancelling any ongoing orchestration") - } - return true + } else if objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() { // Set the cancellation flag to stop any ongoing orchestration rookContext.RequestCancelOrchestration.Set() logger.Infof("CR %q is going be deleted, cancelling any ongoing orchestration", objNew.Name) return true + } else if objOld.GetGeneration() != objNew.GetGeneration() { logger.Debugf("skipping resource %q update with unchanged spec", objNew.Name) } diff --git a/pkg/operator/ceph/controller/spec.go b/pkg/operator/ceph/controller/spec.go index 7bbf040f4..7484ed0af 100644 --- a/pkg/operator/ceph/controller/spec.go +++ b/pkg/operator/ceph/controller/spec.go @@ -385,7 +385,7 @@ func CheckPodMemory(name string, resources v1.ResourceRequirements, cephPodMinim // This means LIMIT and REQUEST are either identical or different but still we use LIMIT as a reference if uint64(podMemoryLimit.Value()) < display.MbTob(cephPodMinimumMemory) { // allow the configuration if less than the min, but print a warning - logger.Warningf("running the %q daemon(s) with %dmb of ram, but at least %dmb is recommended", name, display.BToMb(uint64(podMemoryLimit.Value())), cephPodMinimumMemory) + logger.Warningf("running the %q daemon(s) with %dMB of ram, but at least %dMB is recommended", name, display.BToMb(uint64(podMemoryLimit.Value())), cephPodMinimumMemory) } // This means LIMIT < REQUEST diff --git a/pkg/operator/ceph/csi/spec.go b/pkg/operator/ceph/csi/spec.go index 8e67f45b9..38dd63fa2 100644 --- a/pkg/operator/ceph/csi/spec.go +++ b/pkg/operator/ceph/csi/spec.go @@ -450,7 +450,7 @@ func startDrivers(clientset kubernetes.Interface, rookclientset rookclient.Inter if err != nil { return errors.Wrapf(err, "failed to apply network config to rbd plugin provisioner deployment: %+v", rbdProvisionerDeployment) } - err = k8sutil.CreateDeployment(clientset, csiRBDProvisioner, namespace, rbdProvisionerDeployment) + _, err = k8sutil.CreateOrUpdateDeployment(clientset, rbdProvisionerDeployment) if err != nil { return errors.Wrapf(err, "failed to start rbd provisioner deployment: %+v", rbdProvisionerDeployment) } @@ -510,7 +510,7 @@ func startDrivers(clientset kubernetes.Interface, rookclientset rookclient.Inter if err != nil { return errors.Wrapf(err, "failed to apply network config to cephfs plugin provisioner deployment: %+v", cephfsProvisionerDeployment) } - err = k8sutil.CreateDeployment(clientset, csiCephFSProvisioner, namespace, cephfsProvisionerDeployment) + _, err = k8sutil.CreateOrUpdateDeployment(clientset, cephfsProvisionerDeployment) if err != nil { return errors.Wrapf(err, "failed to start cephfs provisioner deployment: %+v", cephfsProvisionerDeployment) } diff --git a/pkg/operator/ceph/webhook.go b/pkg/operator/ceph/webhook.go index a0633b501..89f1b54fc 100644 --- a/pkg/operator/ceph/webhook.go +++ b/pkg/operator/ceph/webhook.go @@ -130,7 +130,7 @@ func createWebhookDeployment(ctx context.Context, context *clusterd.Context, adm antiAffinity := csi.GetPodAntiAffinity(k8sutil.AppAttr, appName) admissionControllerDeployment := getDeployment(ctx, context, secretVolume, antiAffinity, admissionImage, admission_parameters, secretVolumeMount) - err := k8sutil.CreateDeployment(context.Clientset, appName, namespace, &admissionControllerDeployment) + _, err := k8sutil.CreateOrUpdateDeployment(context.Clientset, &admissionControllerDeployment) if err != nil { return errors.Wrap(err, "failed to create admission-controller deployment") } diff --git a/pkg/operator/k8sutil/daemonset.go b/pkg/operator/k8sutil/daemonset.go index e3d9d2150..d92bfe14a 100644 --- a/pkg/operator/k8sutil/daemonset.go +++ b/pkg/operator/k8sutil/daemonset.go @@ -20,15 +20,14 @@ import ( "context" "fmt" - apps "k8s.io/api/apps/v1" - v1 "k8s.io/api/apps/v1" + appsv1 "k8s.io/api/apps/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) // CreateDaemonSet creates -func CreateDaemonSet(name, namespace string, clientset kubernetes.Interface, ds *apps.DaemonSet) error { +func CreateDaemonSet(name, namespace string, clientset kubernetes.Interface, ds *appsv1.DaemonSet) error { ctx := context.TODO() _, err := clientset.AppsV1().DaemonSets(namespace).Create(ctx, ds, metav1.CreateOptions{}) if err != nil { @@ -57,7 +56,7 @@ func DeleteDaemonset(clientset kubernetes.Interface, namespace, name string) err // AddRookVersionLabelToDaemonSet adds or updates a label reporting the Rook version which last // modified a DaemonSet. -func AddRookVersionLabelToDaemonSet(d *v1.DaemonSet) { +func AddRookVersionLabelToDaemonSet(d *appsv1.DaemonSet) { if d == nil { return } @@ -70,7 +69,7 @@ func AddRookVersionLabelToDaemonSet(d *v1.DaemonSet) { // GetDaemonsets returns a list of daemonsets names labels matching a given selector // example of a label selector might be "app=rook-ceph-mon, mon!=b" // more: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ -func GetDaemonsets(clientset kubernetes.Interface, namespace, labelSelector string) (*apps.DaemonSetList, error) { +func GetDaemonsets(clientset kubernetes.Interface, namespace, labelSelector string) (*appsv1.DaemonSetList, error) { listOptions := metav1.ListOptions{LabelSelector: labelSelector} ctx := context.TODO() daemonsets, err := clientset.AppsV1().DaemonSets(namespace).List(ctx, listOptions) diff --git a/pkg/operator/k8sutil/deployment.go b/pkg/operator/k8sutil/deployment.go index c3e1fa458..f84ed8ad0 100644 --- a/pkg/operator/k8sutil/deployment.go +++ b/pkg/operator/k8sutil/deployment.go @@ -22,10 +22,10 @@ import ( "time" "github.com/banzaicloud/k8s-objectmatcher/patch" - + "github.com/pkg/errors" "github.com/rook/rook/pkg/clusterd" - apps "k8s.io/api/apps/v1" - v1 "k8s.io/api/apps/v1" + "github.com/rook/rook/pkg/util" + appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -33,6 +33,11 @@ import ( "k8s.io/client-go/kubernetes" ) +var ( + waitForDeploymentPeriod = 2 * time.Second + waitForDeploymentTimeout = 60 * time.Second +) + // GetDeploymentImage returns the version of the image running in the pod spec for the desired container func GetDeploymentImage(clientset kubernetes.Interface, namespace, name, container string) (string, error) { ctx := context.TODO() @@ -44,7 +49,7 @@ func GetDeploymentImage(clientset kubernetes.Interface, namespace, name, contain } // GetDeploymentSpecImage returns the image name from the spec -func GetDeploymentSpecImage(clientset kubernetes.Interface, d apps.Deployment, container string, initContainer bool) (string, error) { +func GetDeploymentSpecImage(clientset kubernetes.Interface, d appsv1.Deployment, container string, initContainer bool) (string, error) { image, err := GetSpecContainerImage(d.Spec.Template.Spec, container, initContainer) if err != nil { return "", err @@ -61,7 +66,7 @@ func GetDeploymentSpecImage(clientset kubernetes.Interface, d apps.Deployment, c // 2. verify that we can continue the update procedure // Basically, we go one resource by one and check if we can stop and then if the resource has been successfully updated // we check if we can go ahead and move to the next one. -func UpdateDeploymentAndWait(clusterContext *clusterd.Context, modifiedDeployment *apps.Deployment, namespace string, verifyCallback func(action string) error) error { +func UpdateDeploymentAndWait(clusterContext *clusterd.Context, modifiedDeployment *appsv1.Deployment, namespace string, verifyCallback func(action string) error) error { ctx := context.TODO() currentDeployment, err := clusterContext.Clientset.AppsV1().Deployments(namespace).Get(ctx, modifiedDeployment.Name, metav1.GetOptions{}) if err != nil { @@ -111,7 +116,7 @@ func UpdateDeploymentAndWait(clusterContext *clusterd.Context, modifiedDeploymen return nil } -func WaitForDeploymentToStart(clusterdContext *clusterd.Context, deployment *v1.Deployment) error { +func WaitForDeploymentToStart(clusterdContext *clusterd.Context, deployment *appsv1.Deployment) error { // wait for the deployment to be restarted up to 300s sleepTime := 3 attempts := 100 @@ -129,7 +134,7 @@ func WaitForDeploymentToStart(clusterdContext *clusterd.Context, deployment *v1. // If ProgressDeadlineExceeded is reached let's fail earlier // This can happen if one of the deployment cannot be scheduled on a node and stays in "pending" state for _, condition := range d.Status.Conditions { - if condition.Type == v1.DeploymentProgressing && condition.Reason == "ProgressDeadlineExceeded" { + if condition.Type == appsv1.DeploymentProgressing && condition.Reason == "ProgressDeadlineExceeded" { return fmt.Errorf("gave up waiting for deployment %q to update because %q", deployment.Name, condition.Reason) } } @@ -140,10 +145,232 @@ func WaitForDeploymentToStart(clusterdContext *clusterd.Context, deployment *v1. return fmt.Errorf("gave up waiting for deployment %q to update", deployment.Name) } +// DeploymentNames returns a list of the names of deployments in the deployment list +func DeploymentNames(deployments *appsv1.DeploymentList) (names []string) { + for _, d := range deployments.Items { + names = append(names, d.Name) + } + return names +} + +// DeploymentsUpdated is a mapping from deployment name to the observed generation of the old +// deployment which was updated. +type DeploymentsUpdated map[string]int64 + +// Names returns the names of the deployments which were updated. +func (d *DeploymentsUpdated) Names() (names []string) { + for name := range *d { + names = append(names, name) + } + return names +} + +type Failure struct { + ResourceName string + Error error +} + +type Failures []Failure + +func (failures *Failures) CollatedErrors() error { + var err error + for _, f := range *failures { + err = fmt.Errorf("%v. %v", f.Error, err) + } + return err +} + +// UpdateMultipleDeployments updates multiple deployments and returns DeploymentsUpdated map of +// deployment names which were updated successfully and the observed generation of the deployments +// before they were updated. If deployments are already up to date, they are not reported in the +// DeploymentsUpdated map. +// The DeploymentsUpdated map can be used with the WaitForDeploymentsToUpdate function. +// Also returns a list of failures. Each failure returned includes the name of the deployment which +// could not be updated and the error experienced when attempting to update the deployment. +func UpdateMultipleDeployments( + clientset kubernetes.Interface, + deployments []*appsv1.Deployment, +) (DeploymentsUpdated, Failures, *int32) { + deploymentsUpdated := DeploymentsUpdated{} + failures := Failures{} + var maxProgressDeadlineSeconds *int32 + + for _, dep := range deployments { + oldDep, newDep, err := updateDeployment(clientset, dep) + if err != nil { + failures = append(failures, Failure{ + ResourceName: dep.Name, + Error: errors.Wrapf(err, "failed to update deployment %q", dep.Name), + }) + continue + } + + if newDep == nil { + // deployment was not updated + continue + } + + deploymentsUpdated[newDep.Name] = oldDep.Status.ObservedGeneration + + maxProgressDeadlineSeconds = maxInt32Ptr(maxProgressDeadlineSeconds, oldDep.Spec.ProgressDeadlineSeconds) + maxProgressDeadlineSeconds = maxInt32Ptr(maxProgressDeadlineSeconds, newDep.Spec.ProgressDeadlineSeconds) + } + + return deploymentsUpdated, failures, maxProgressDeadlineSeconds +} + +// WaitForDeploymentsToUpdate waits for all deployments to update. It returns a list of failures. +// Each failure includes the name of the deployment which was not updated within the timeout and an +// error indicating the reason why. +func WaitForDeploymentsToUpdate( + deploymentsUpdated DeploymentsUpdated, + progressDeadlineSeconds *int32, + listFunc func() (*appsv1.DeploymentList, error), +) Failures { + // do not modify the input! + waitingOn := DeploymentsUpdated{} + for k, v := range deploymentsUpdated { + waitingOn[k] = v + } + failures := Failures{} + + waitFunc := func() (done bool, err error) { + deployments, err := listFunc() + if err != nil { + return false, errors.Wrap(err, "failed to list deployments") + } + if len(deployments.Items) < len(waitingOn) { + // This could be because the listFunc is written incorrectly + logger.Warningf( + "number of deployments listed (%d) is less than number of deployments we are waiting on to be updated (%d). listed: %+v. waiting on: %+v. ", + len(deployments.Items), len(waitingOn), DeploymentNames(deployments), waitingOn.Names()) + } + + for i, dep := range deployments.Items { // deployment loop + oldGeneration, ok := waitingOn[dep.Name] + if !ok { + // we are not waiting on this deployment to finish + continue + } + + // If ProgressDeadlineExceeded is reached, fail earlier. This can happen if a deployment + // cannot be scheduled on a node and stays in "pending" state. + // Index deployment to prevent implicit memory aliasing + if err := progressDeadlineExceeded(&deployments.Items[i]); err != nil { + failures = append(failures, Failure{ + ResourceName: dep.Name, + Error: errors.Wrapf(err, "progress deadline exceeded on deployment %q", dep.Name), + }) + delete(waitingOn, dep.Name) // don't keep waiting on this deployment + continue // deployment loop + } + + // Index deployment to prevent implicit memory aliasing + if deploymentIsDoneUpdating(&deployments.Items[i], oldGeneration) { + delete(waitingOn, dep.Name) // done waiting on this deployment + } + } + + if len(waitingOn) == 0 { + return true, nil + } + + return false, nil + } + + timeout := waitForDeploymentTimeout + if progressDeadlineSeconds != nil { + // make the timeout double the progress deadline since the pod is both stopping and starting + timeout = 2 * time.Duration(*progressDeadlineSeconds) * time.Second + } + + err := util.RetryWithTimeout(waitFunc, waitForDeploymentPeriod, timeout, "deployments to be updated") + if err != nil { + // the retry function doesn't return (true, error), so this must be a timeout error + logger.Errorf("%v", err) + } + + // process remaining items in the waitingOn list to mark which deployments timed out + for depName := range waitingOn { + failures = append(failures, Failure{ + ResourceName: depName, + Error: errors.Errorf("timed out waiting on deployment %q", depName), + }) + } + + return failures +} + +func UpdateMultipleDeploymentsAndWait( + clientset kubernetes.Interface, + deployments []*appsv1.Deployment, + listFunc func() (*appsv1.DeploymentList, error), +) Failures { + depsUpdated, updateFailures, maxProgressDeadline := UpdateMultipleDeployments(clientset, deployments) + waitFailures := WaitForDeploymentsToUpdate(depsUpdated, maxProgressDeadline, listFunc) + + return append(updateFailures, waitFailures...) +} + +func deploymentIsDoneUpdating(d *appsv1.Deployment, oldObservedGeneration int64) bool { + return d.Status.ObservedGeneration != oldObservedGeneration && d.Status.UpdatedReplicas > 0 && d.Status.ReadyReplicas > 0 +} + +// return error if progress deadline exceeded condition on the deployment +func progressDeadlineExceeded(d *appsv1.Deployment) error { + for _, condition := range d.Status.Conditions { + if condition.Type == appsv1.DeploymentProgressing && condition.Reason == "ProgressDeadlineExceeded" { + return fmt.Errorf("gave up waiting for deployment %q to update because %q", d.Name, condition.Reason) + } + } + return nil +} + +func updateDeployment( + clientset kubernetes.Interface, + deployment *appsv1.Deployment, +) (oldDeployment, newDeployment *appsv1.Deployment, err error) { + ctx := context.TODO() + namespace := deployment.Namespace + + oldDeployment, err = clientset.AppsV1().Deployments(namespace).Get(ctx, deployment.Name, metav1.GetOptions{}) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to get existing deployment %q", deployment.Name) + } + + // Check whether the current deployment and newly generated one are identical + patchChanged := false + patchResult, err := patch.DefaultPatchMaker.Calculate(oldDeployment, deployment) + if err != nil { + logger.Warningf("failed to calculate diff between current deployment %q and newly generated one. assuming it changed. %v", oldDeployment.Name, err) + patchChanged = true + } else if !patchResult.IsEmpty() { + patchChanged = true + } + + if patchChanged { + // Set hash annotation to the newly generated deployment + err := patch.DefaultAnnotator.SetLastAppliedAnnotation(deployment) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to set hash annotation on deployment %q", deployment.Name) + } + + newDeployment, err := clientset.AppsV1().Deployments(namespace).Update(ctx, deployment, metav1.UpdateOptions{}) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to update deployment %q", deployment.Name) + } + + return oldDeployment, newDeployment, nil + } + + logger.Debugf("deployment %q did not change. nothing to update", deployment.Name) + return oldDeployment, nil, nil +} + // GetDeployments returns a list of deployment names labels matching a given selector // example of a label selector might be "app=rook-ceph-mon, mon!=b" // more: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ -func GetDeployments(clientset kubernetes.Interface, namespace, labelSelector string) (*apps.DeploymentList, error) { +func GetDeployments(clientset kubernetes.Interface, namespace, labelSelector string) (*appsv1.DeploymentList, error) { listOptions := metav1.ListOptions{LabelSelector: labelSelector} ctx := context.TODO() deployments, err := clientset.AppsV1().Deployments(namespace).List(ctx, listOptions) @@ -208,7 +435,7 @@ func WaitForDeploymentImage(clientset kubernetes.Interface, namespace, label, co // AddRookVersionLabelToDeployment adds or updates a label reporting the Rook version which last // modified a deployment. -func AddRookVersionLabelToDeployment(d *v1.Deployment) { +func AddRookVersionLabelToDeployment(d *appsv1.Deployment) { if d == nil { return } @@ -225,7 +452,7 @@ func AddRookVersionLabelToObjectMeta(meta *metav1.ObjectMeta) { addRookVersionLabel(meta.Labels) } -func AddLabelToDeployment(key, value string, d *v1.Deployment) { +func AddLabelToDeployment(key, value string, d *appsv1.Deployment) { if d == nil { return } @@ -259,16 +486,52 @@ func addLabel(key, value string, labels map[string]string) { labels[key] = value } -func CreateDeployment(clientset kubernetes.Interface, name, namespace string, dep *apps.Deployment) error { +// CreateDeployment creates a deployment with a last applied hash annotation added +func CreateDeployment(clientset kubernetes.Interface, dep *appsv1.Deployment) (*appsv1.Deployment, error) { ctx := context.TODO() - _, err := clientset.AppsV1().Deployments(namespace).Create(ctx, dep, metav1.CreateOptions{}) + + // Set hash annotation to the newly generated deployment + err := patch.DefaultAnnotator.SetLastAppliedAnnotation(dep) + if err != nil { + return nil, errors.Wrapf(err, "failed to set hash annotation on deployment %q", dep.Name) + } + + return clientset.AppsV1().Deployments(dep.Namespace).Create(ctx, dep, metav1.CreateOptions{}) +} + +func CreateOrUpdateDeployment(clientset kubernetes.Interface, dep *appsv1.Deployment) (*appsv1.Deployment, error) { + ctx := context.TODO() + + newDep, err := CreateDeployment(clientset, dep) if err != nil { if k8serrors.IsAlreadyExists(err) { - _, err = clientset.AppsV1().Deployments(namespace).Update(ctx, dep, metav1.UpdateOptions{}) + // annotation was added in CreateDeployment to dep passed by reference + newDep, err = clientset.AppsV1().Deployments(dep.Namespace).Update(ctx, dep, metav1.UpdateOptions{}) } if err != nil { - return fmt.Errorf("failed to start %s deployment: %+v\n%+v", name, err, dep) + return nil, errors.Wrapf(err, "failed to create or update deployment %q: %+v", dep.Name, dep) } } - return err + return newDep, nil +} + +func maxInt32Ptr(a, b *int32) *int32 { + var ret int32 + if a == nil && b == nil { + return nil + } + if a == nil && b != nil { + ret = *b + return &ret + } + if a != nil && b == nil { + ret = *a + return &ret + } + if *b > *a { + ret = *b + return &ret + } + ret = *a + return &ret } diff --git a/pkg/operator/k8sutil/deployment_test.go b/pkg/operator/k8sutil/deployment_test.go new file mode 100644 index 000000000..0907d8ec7 --- /dev/null +++ b/pkg/operator/k8sutil/deployment_test.go @@ -0,0 +1,482 @@ +/* +Copyright 2018 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package k8sutil + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" +) + +func TestUpdateMultipleDeploymentsAndWait(t *testing.T) { + // UpdateMultipleDeployments and WaitForDeploymentsToUpdate are thoroughly tested + // UpdateMultipleDeploymentsAndWait is a simple wrapper for both of their functionality + // Merely do basic integration testing on this to ensure they are connected properly + + namespace := "ns" + + oldPeriod := waitForDeploymentPeriod + oldTimeout := waitForDeploymentTimeout + defer func() { + waitForDeploymentPeriod = oldPeriod + waitForDeploymentTimeout = oldTimeout + }() + waitForDeploymentPeriod = 1 * time.Millisecond + waitForDeploymentTimeout = 3 * time.Millisecond + + // inputs + var clientset *fake.Clientset + var deployments []*appsv1.Deployment + var listFunc func() (*appsv1.DeploymentList, error) + + // outputs + var failures Failures + + // a barebones deployment + deployment := func(name string) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + } + // a barebones deployment with some modification + modifiedDeployment := func(name string) *appsv1.Deployment { + d := deployment(name) + d.Spec.Selector = &metav1.LabelSelector{ + MatchLabels: map[string]string{"hi": "mom"}, + } + return d + } + + timesCalled := 0 + // generate a status that is not ready when first called but becomes ready later + status := func(timesCalled int) appsv1.DeploymentStatus { + if timesCalled < 0 { + timesCalled = 0 + } + return appsv1.DeploymentStatus{ + ObservedGeneration: int64(timesCalled), + UpdatedReplicas: int32(timesCalled), + ReadyReplicas: int32(timesCalled), + Conditions: []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentProgressing, Reason: "DoinStuff"}, + }, + } + } + // generate a deployment with no errors and status=status(timesCalled) + okayDeployment := func(name string, timesCalled int) appsv1.Deployment { + return appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Status: status(timesCalled), + } + } + addProgressDeadlineExceeded := func(d *appsv1.Deployment) { + d.Status.Conditions = append(d.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentProgressing, Reason: "ProgressDeadlineExceeded"}) + } + + clientset = fake.NewSimpleClientset() + depsUpdated := []string{} + var deploymentReactor k8stesting.ReactionFunc = func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + switch action := action.(type) { + case k8stesting.UpdateActionImpl: + obj := action.GetObject() + d, ok := obj.(*appsv1.Deployment) + if !ok { + panic("err! action not a deployment") + } + depsUpdated = append(depsUpdated, d.Name) + default: + panic(fmt.Sprintf("action %v not understood", action)) + } + return false, nil, nil + } + clientset.PrependReactor("update", "deployments", deploymentReactor) + + t.Run("integration", func(t *testing.T) { + createDeploymentOrDie(clientset, deployment("d1")) + createDeploymentOrDie(clientset, deployment("d2")) + createDeploymentOrDie(clientset, deployment("d3")) + createDeploymentOrDie(clientset, deployment("d4")) + deployments = []*appsv1.Deployment{ + modifiedDeployment("d1"), // should be updated (and successfully) + modifiedDeployment("d2"), // should be updated (but gets progress deadline exceeded) + modifiedDeployment("d3"), // should be updated (but never becomes ready) + deployment("d4"), // no changes so should not be updated + deployment("d5"), // does not exist so should be a failure to update + } + listFunc = func() (*appsv1.DeploymentList, error) { + d2 := okayDeployment("d2", 0) + if timesCalled >= 1 { + addProgressDeadlineExceeded(&d2) + } + l := &appsv1.DeploymentList{ + Items: []appsv1.Deployment{ + okayDeployment("d1", timesCalled), // becomes ready second time called + d2, + okayDeployment("d3", 0), // never becomes ready + }, + } + timesCalled++ + return l, nil + } + failures = UpdateMultipleDeploymentsAndWait(clientset, deployments, listFunc) + assert.Len(t, failures, 3) + assert.ElementsMatch(t, + []string{failures[0].ResourceName, failures[1].ResourceName, failures[2].ResourceName}, + []string{"d2", "d3", "d5"}) + assert.ElementsMatch(t, depsUpdated, []string{"d1", "d2", "d3"}) + }) +} + +func TestUpdateMultipleDeployments(t *testing.T) { + namespace := "ns" + + // a barebones deployment + deployment := func(name string, pds *int32) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + ProgressDeadlineSeconds: pds, + }, + } + } + // a barebones deployment with some modification + modifiedDeployment := func(name string, pds *int32) *appsv1.Deployment { + d := deployment(name, pds) + d.Spec.Selector = &metav1.LabelSelector{ + MatchLabels: map[string]string{"hi": "mom"}, + } + return d + } + + // inputs + var clientset *fake.Clientset + var deployments []*appsv1.Deployment + + // outputs + var deploymentsUpdated DeploymentsUpdated + var failures Failures + var pds *int32 // progressDeadlineSeconds + + t.Run("no deployments to be updated", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + deployments = []*appsv1.Deployment{} + deploymentsUpdated, failures, pds = UpdateMultipleDeployments(clientset, deployments) + assert.Len(t, deploymentsUpdated, 0) + assert.Len(t, failures, 0) + }) + + t.Run("update deployments successfully", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + createDeploymentOrDie(clientset, deployment("d1", nil)) + createDeploymentOrDie(clientset, deployment("d2", nil)) + createDeploymentOrDie(clientset, deployment("d3", nil)) + deployments = []*appsv1.Deployment{ + modifiedDeployment("d1", nil), + modifiedDeployment("d2", nil), + modifiedDeployment("d3", nil), + } + deploymentsUpdated, failures, pds = UpdateMultipleDeployments(clientset, deployments) + assert.Len(t, deploymentsUpdated, 3) + assert.Len(t, failures, 0) + assert.Contains(t, deploymentsUpdated, "d1") + assert.Contains(t, deploymentsUpdated, "d2") + assert.Contains(t, deploymentsUpdated, "d3") + assert.Nil(t, pds) + }) + + t.Run("do not update deployments with no modifications", func(t *testing.T) { + // clientset with preexisting and updated deployments from previous test + d3 := modifiedDeployment("d3", newInt32(5)) + // d3.ObjectMeta.Labels = map[string]string{ + // "new": "label", + // } + deployments = []*appsv1.Deployment{ + modifiedDeployment("d1", nil), // should not be updated + // d2 from before should also not be updated + d3, // should be updated + } + deploymentsUpdated, failures, pds = UpdateMultipleDeployments(clientset, deployments) + assert.Len(t, deploymentsUpdated, 1) + assert.Len(t, failures, 0) + assert.Contains(t, deploymentsUpdated, "d3") + assert.EqualValues(t, 5, *pds) + }) + + t.Run("failures if deployments do not exist", func(t *testing.T) { + clientset = fake.NewSimpleClientset() + createDeploymentOrDie(clientset, deployment("d1", newInt32(30))) + createDeploymentOrDie(clientset, deployment("d3", newInt32(30))) + deployments = []*appsv1.Deployment{ + modifiedDeployment("d1", newInt32(30)), + modifiedDeployment("d2", newInt32(30)), + modifiedDeployment("d3", newInt32(30)), + modifiedDeployment("d4", newInt32(30)), + } + deploymentsUpdated, failures, pds = UpdateMultipleDeployments(clientset, deployments) + assert.Len(t, deploymentsUpdated, 2) + assert.Len(t, failures, 2) + assert.Contains(t, deploymentsUpdated, "d1") + assert.Contains(t, deploymentsUpdated, "d3") + assert.ElementsMatch(t, []string{failures[0].ResourceName, failures[1].ResourceName}, []string{"d2", "d4"}) + assert.EqualValues(t, 30, *pds) + }) +} + +func TestWaitForDeploymentsToUpdate(t *testing.T) { + // NOTE: do not actually test progressDeadlineExceeded as an input because we don't want to make + // this unit test run for multiple seconds. + + oldPeriod := waitForDeploymentPeriod + oldTimeout := waitForDeploymentTimeout + defer func() { + waitForDeploymentPeriod = oldPeriod + waitForDeploymentTimeout = oldTimeout + }() + waitForDeploymentPeriod = 1 * time.Millisecond + waitForDeploymentTimeout = 3 * time.Millisecond + + timesCalled := 0 + // generate a status that is not ready when first called but becomes ready later + status := func(timesCalled int) appsv1.DeploymentStatus { + if timesCalled < 0 { + timesCalled = 0 + } + return appsv1.DeploymentStatus{ + ObservedGeneration: int64(timesCalled), + UpdatedReplicas: int32(timesCalled), + ReadyReplicas: int32(timesCalled), + Conditions: []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentProgressing, Reason: "DoinStuff"}, + }, + } + } + // generate a deployment with no errors and status=status(timesCalled) + okayDeployment := func(name string, timesCalled int) appsv1.Deployment { + return appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Status: status(timesCalled), + } + } + addProgressDeadlineExceeded := func(d *appsv1.Deployment) { + d.Status.Conditions = append(d.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentProgressing, Reason: "ProgressDeadlineExceeded"}) + } + + // inputs + var deploymentsUpdated DeploymentsUpdated + var progressDeadlineSeconds *int32 + var listFunc func() (*appsv1.DeploymentList, error) + + // outputs + var failures Failures + + runTest := func() { + timesCalled = 0 + failures = WaitForDeploymentsToUpdate(deploymentsUpdated, progressDeadlineSeconds, listFunc) + } + + t.Run("not waiting on any deployments", func(t *testing.T) { + deploymentsUpdated = DeploymentsUpdated{} + listFunc = func() (*appsv1.DeploymentList, error) { + l := &appsv1.DeploymentList{ + Items: []appsv1.Deployment{ + okayDeployment("d1", timesCalled), // becomes ready second time called + okayDeployment("d2", timesCalled-1), // becomes ready third time called + okayDeployment("d3", timesCalled), // becomes ready second time called but only generation changed on third time + }, + } + timesCalled++ + return l, nil + } + runTest() + assert.Len(t, failures, 0) + }) + + t.Run("wait successful", func(t *testing.T) { + deploymentsUpdated = DeploymentsUpdated{ + "d1": 0, + "d2": 0, + "d3": 1, + } + listFunc = func() (*appsv1.DeploymentList, error) { + l := &appsv1.DeploymentList{ + Items: []appsv1.Deployment{ + okayDeployment("d1", timesCalled), // becomes ready second time called + okayDeployment("d2", timesCalled-1), // becomes ready third time called + okayDeployment("d3", timesCalled), // becomes ready second time called but only generation changed on third time + }, + } + timesCalled++ + return l, nil + } + runTest() + assert.Len(t, failures, 0) + }) + + t.Run("some failures", func(t *testing.T) { + deploymentsUpdated = DeploymentsUpdated{ + "d1": 0, + "d2": 0, + "d3": 0, + } + listFunc = func() (*appsv1.DeploymentList, error) { + l := &appsv1.DeploymentList{ + Items: []appsv1.Deployment{ + okayDeployment("d1", timesCalled), // becomes ready second time called + okayDeployment("d2", 0), // never becomes ready + okayDeployment("d3", 0), // never becomes ready + }, + } + timesCalled++ + return l, nil + } + runTest() + assert.Len(t, failures, 2) + assert.ElementsMatch(t, []string{failures[0].ResourceName, failures[1].ResourceName}, []string{"d2", "d3"}) + assert.Error(t, failures[0].Error) + assert.Error(t, failures[1].Error) + }) + + t.Run("one deployment never listed", func(t *testing.T) { + deploymentsUpdated = DeploymentsUpdated{ + "d1": 0, + "d2": 0, + "d3": 0, + } + listFunc = func() (*appsv1.DeploymentList, error) { + l := &appsv1.DeploymentList{ + Items: []appsv1.Deployment{ + okayDeployment("d1", timesCalled), // becomes ready second time called + okayDeployment("d2", timesCalled-1), // becomes ready third time called + }, + } + timesCalled++ + return l, nil + } + runTest() + assert.Len(t, failures, 1) + assert.ElementsMatch(t, []string{failures[0].ResourceName}, []string{"d3"}) + assert.Error(t, failures[0].Error) + }) + + t.Run("fail to list deployments", func(t *testing.T) { + deploymentsUpdated = DeploymentsUpdated{ + "d1": 0, + "d2": 0, + "d3": 0, + } + listFunc = func() (*appsv1.DeploymentList, error) { + return nil, fmt.Errorf("fake error listing deployments") + } + runTest() + assert.Len(t, failures, 3) + assert.ElementsMatch(t, + []string{failures[0].ResourceName, failures[1].ResourceName, failures[2].ResourceName}, + []string{"d1", "d2", "d3"}) + assert.Error(t, failures[0].Error) + assert.Error(t, failures[1].Error) + assert.Error(t, failures[2].Error) + }) + + t.Run("deployment progress deadline exceeded", func(t *testing.T) { + deploymentsUpdated = DeploymentsUpdated{ + "d1": 0, + "d2": 0, + "d3": 0, + } + listFunc = func() (*appsv1.DeploymentList, error) { + d2 := okayDeployment("d2", 0) + d3 := okayDeployment("d3", 0) + if timesCalled > 2 { + addProgressDeadlineExceeded(&d2) + addProgressDeadlineExceeded(&d3) + } + l := &appsv1.DeploymentList{ + Items: []appsv1.Deployment{ + okayDeployment("d1", timesCalled), // becomes ready second time called + d2, + d3, + }, + } + timesCalled++ + return l, nil + } + runTest() + assert.Len(t, failures, 2) + assert.ElementsMatch(t, []string{failures[0].ResourceName, failures[1].ResourceName}, []string{"d2", "d3"}) + assert.Error(t, failures[0].Error) + assert.Error(t, failures[1].Error) + }) +} + +func Test_maxInt32Ptr(t *testing.T) { + t.Run("both nil", func(t *testing.T) { + assert.Nil(t, maxInt32Ptr(nil, nil)) + }) + + type args struct { + a *int32 + b *int32 + } + tests := []struct { + name string + args args + want int32 + }{ + {"a nil", args{nil, newInt32(2)}, 2}, + {"b nil", args{newInt32(0), nil}, 0}, + {"negatives", args{newInt32(-5), newInt32(-4)}, -4}, + {"neg and pos", args{newInt32(-3), newInt32(1)}, 1}, + {"positives", args{newInt32(1), newInt32(3)}, 3}, + {"with zero", args{newInt32(0), newInt32(2)}, 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, *maxInt32Ptr(tt.args.a, tt.args.b)) + }) + } +} + +func newInt32(i int32) *int32 { + return &i +} + +func createDeploymentOrDie(clientset *fake.Clientset, d *appsv1.Deployment) { + _, err := clientset.AppsV1().Deployments(d.Namespace).Create(context.TODO(), d, metav1.CreateOptions{}) + if err != nil { + panic(err) + } +} diff --git a/pkg/operator/k8sutil/job.go b/pkg/operator/k8sutil/job.go index 5c34b1b5d..acfb27045 100644 --- a/pkg/operator/k8sutil/job.go +++ b/pkg/operator/k8sutil/job.go @@ -76,7 +76,7 @@ func WaitForJobCompletion(clientset kubernetes.Interface, job *batch.Job, timeou return false, nil } if job.Status.Failed > 0 { - return false, fmt.Errorf("job %s failed.", job.Name) + return false, fmt.Errorf("job %s failed", job.Name) } if job.Status.Succeeded > 0 { return true, nil diff --git a/pkg/operator/k8sutil/k8sutil.go b/pkg/operator/k8sutil/k8sutil.go index 2751332bc..d3c5b990c 100644 --- a/pkg/operator/k8sutil/k8sutil.go +++ b/pkg/operator/k8sutil/k8sutil.go @@ -159,6 +159,18 @@ func addRookVersionLabel(labels map[string]string) { labels[RookVersionLabelKey] = value } +// RookVersionLabelMatchesCurrent returns true if the Rook version on the resource label matches the +// current Rook version running. It returns false if the label does not match. It returns an error +// if the label does not exist. +func RookVersionLabelMatchesCurrent(labels map[string]string) (bool, error) { + v, ok := labels[RookVersionLabelKey] + if !ok { + return false, fmt.Errorf("failed to find Rook version label %q", RookVersionLabelKey) + } + expectedVersion := validateLabelValue(rookversion.Version) + return (v == expectedVersion), nil +} + // validateLabelValue replaces any invalid characters // in the input string with a replacement character, // and enforces other limitations for k8s label values. diff --git a/pkg/operator/k8sutil/prometheus.go b/pkg/operator/k8sutil/prometheus.go index ce3fc869c..cf27f074f 100644 --- a/pkg/operator/k8sutil/prometheus.go +++ b/pkg/operator/k8sutil/prometheus.go @@ -26,8 +26,7 @@ import ( monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" monitoringclient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned" - "k8s.io/apimachinery/pkg/api/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" + kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sYAML "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/client-go/tools/clientcmd" @@ -71,7 +70,7 @@ func CreateOrUpdateServiceMonitor(serviceMonitorDefinition *monitoringv1.Service } oldSm, err := client.MonitoringV1().ServiceMonitors(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { - if apierrors.IsNotFound(err) { + if kerrors.IsNotFound(err) { sm, err := client.MonitoringV1().ServiceMonitors(namespace).Create(ctx, serviceMonitorDefinition, metav1.CreateOptions{}) if err != nil { return nil, fmt.Errorf("failed to create servicemonitor. %v", err) @@ -114,7 +113,7 @@ func CreateOrUpdatePrometheusRule(prometheusRule *monitoringv1.PrometheusRule) ( } promRule, err := client.MonitoringV1().PrometheusRules(namespace).Create(ctx, prometheusRule, metav1.CreateOptions{}) if err != nil { - if !errors.IsAlreadyExists(err) { + if !kerrors.IsAlreadyExists(err) { return nil, fmt.Errorf("failed to create prometheusRules. %v", err) } // Get current PrometheusRule so the ResourceVersion can be set as needed diff --git a/pkg/operator/test/client.go b/pkg/operator/test/client.go index b4a3aec79..b97c8e2db 100644 --- a/pkg/operator/test/client.go +++ b/pkg/operator/test/client.go @@ -20,7 +20,6 @@ import ( "context" "encoding/base32" "fmt" - "math/rand" "strings" "testing" @@ -41,6 +40,7 @@ import ( // New creates a fake K8s cluster with some nodes added. func New(t *testing.T, nodes int) *fake.Clientset { + t.Helper() clientset := fake.NewSimpleClientset() AddSomeReadyNodes(t, clientset, nodes) return clientset @@ -48,9 +48,13 @@ func New(t *testing.T, nodes int) *fake.Clientset { // AddReadyNode adds a new Node with status "Ready" and the given name and IP. func AddReadyNode(t *testing.T, clientset *fake.Clientset, name, ip string) { + t.Helper() ready := v1.NodeCondition{Type: v1.NodeReady, Status: v1.ConditionTrue} n := &v1.Node{ ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1.LabelHostname: name, + }, Name: name, }, Status: v1.NodeStatus{ @@ -78,6 +82,7 @@ func AddReadyNode(t *testing.T, clientset *fake.Clientset, name, ip string) { // - name from 0 to count-1 // - ip from 0.0.0.0 to ... func AddSomeReadyNodes(t *testing.T, clientset *fake.Clientset, count int) { + t.Helper() for i := 0; i < count; i++ { name := fmt.Sprintf("node%d", i) ip := fmt.Sprintf("%d.%d.%d.%d", i, i, i, i) @@ -123,6 +128,7 @@ var ( // to the clientset to mimic more of what K8s does in the real world. // - Generate a name for resources that have 'generateName' set and 'name' unset. func NewComplexClientset(t *testing.T) *fake.Clientset { + t.Helper() clientset := fake.NewSimpleClientset() // Some resources are created with generateName used, and we need to capture the create @@ -169,6 +175,7 @@ func NewComplexClientset(t *testing.T) *fake.Clientset { // - Pod create/delete is done to the clientset tracker, so no Pod watch events will register. // - Optionally look through the clientset Nodes to randomly assign created Pods to a node. func PrependComplexJobReactor(t *testing.T, clientset *fake.Clientset, assignPodToNode bool) { + t.Helper() var jobReactor k8stesting.ReactionFunc = func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { switch action := action.(type) { @@ -228,7 +235,18 @@ func PrependComplexJobReactor(t *testing.T, clientset *fake.Clientset, assignPod clientset.PrependReactor("*", "jobs", jobReactor) } -// Pick a random node name from the nodes present in the fake K8s clientset cluster. +// PrependFailReactor adds a reactor with the desired verb and resource that will report a failure. +func PrependFailReactor(t *testing.T, clientset *fake.Clientset, verb, resource string) { + t.Helper() + var failReactor k8stesting.ReactionFunc = func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, fmt.Errorf("fail reactor: induced failure") + } + clientset.PrependReactor(verb, resource, failReactor) +} + +var pickNodeIdx = 0 // global, used to allow round-robin node picking + +// Pick a node name from the nodes present in the fake K8s clientset cluster. func pickNode(clientset *fake.Clientset) string { obj, err := clientset.Tracker().List(nodeGVR, nodeGVK, corev1.NamespaceAll) if err != nil { @@ -241,7 +259,8 @@ func pickNode(clientset *fake.Clientset) string { if len(nodes.Items) == 0 { panic(fmt.Errorf("pickNode: no nodes are available in the fake clientset to pick from")) } - // #nosec G404 It's harmless since it's a test code - randIdx := rand.Intn(len(nodes.Items)) - return nodes.Items[randIdx].GetName() + pickNodeIdx = pickNodeIdx % len(nodes.Items) // reset to 0 once idx is more than number of nodes + name := nodes.Items[pickNodeIdx].GetName() + pickNodeIdx++ + return name } diff --git a/pkg/util/retry.go b/pkg/util/retry.go index a3c02a511..3c15141ea 100644 --- a/pkg/util/retry.go +++ b/pkg/util/retry.go @@ -19,6 +19,8 @@ package util import ( "fmt" "time" + + "github.com/pkg/errors" ) // Retry will attempt the given function until it succeeds, up to the given maximum amount of retries, @@ -41,3 +43,46 @@ func Retry(maxRetries int, delay time.Duration, f func() error) error { <-time.After(delay) } } + +// RetryFunc is a function that returns true when it is done and it should be retried no longer. +// It should return error if there has been an error. The error will be logged if done==false +// (should keep retrying). The error will be returned by the calling function if done==true (should +// stop retrying). +type RetryFunc func() (done bool, err error) + +// RetryWithTimeout retries the RetryFunc until the timeout occurs. It will retry once more after +// the timeout to avoid race conditions at the expense of running for slightly longer than timeout +// in the timeout/error case. +// The given description will be output in log/error messages as "... waiting for ..." +func RetryWithTimeout(f RetryFunc, period time.Duration, timeout time.Duration, description string) error { + tt := time.After(timeout) + var done bool + var err error + for { + done, err = f() + if done { + break + } + if err != nil { + logger.Errorf("error occurred waiting for %s. retrying after %v. %v", description, period, err) + } + logger.Debugf("waiting for %s. retrying after %v", description, period) + + select { + case <-time.After(period): + // go back to start of loop + case <-tt: + // timeout; try one last time + done, err = f() + if !done { + if err != nil { + return errors.Wrapf(err, "timed out waiting for %s", description) + } + return fmt.Errorf("timed out waiting for %s", description) + } + return err + } + } + + return err +} diff --git a/pkg/util/set.go b/pkg/util/set.go index 4f8b447df..2801b59fa 100644 --- a/pkg/util/set.go +++ b/pkg/util/set.go @@ -68,7 +68,7 @@ func (s *Set) Add(newValue string) bool { return false } -// Add a value to the set. Returns true if the value was added, false if it already exists. +// Remove a value from the set. Returns true if the value was removed, false if it does not exist. func (s *Set) Remove(oldValue string) bool { if _, ok := s.values[oldValue]; ok { delete(s.values, oldValue)