Merge pull request canonical#321 from UtkarshBhatthere/feature/prohib…

…itScaledown

Add flag to prohibit scaledown of crush rules on disk removal.

.github/workflows/tests.yml

@@ -307,6 +307,18 @@ jobs:
     - name: Enable services on wrk1
       run: ~/actionutils.sh headexec enable_services node-wrk1
 
+    - name: Remove OSD again but don't scaledown crush rule.
+      run: |
+        set -uex
+        lxc exec node-wrk0 -- sh -c "microceph disk remove 3 --prohibit-crush-scaledown --bypass-safety-checks"
+        lxc exec node-wrk0 -- sh -c "microceph.ceph osd crush rule ls" | grep -F microceph_auto_host
+
+    - name: Re-Add OSD
+      run: |
+        set -uex
+        ~/actionutils.sh add_osd_to_node node-wrk0
+        ~/actionutils.sh headexec wait_for_osds 3
+
     - name: Test remove node wrk3
       run: |
         set -uex

microceph/api/disks.go

@@ -119,18 +119,22 @@ func cmdDisksDelete(s *state.State, r *http.Request) response.Response {
 	defer mu.Unlock()
 
 	cs := interfaces.CephState{State: s}
-	needDowngrade, err := ceph.IsDowngradeNeeded(cs, osdid)
-	if err != nil {
-		return response.InternalError(err)
-	}
-	if needDowngrade && !req.ConfirmDowngrade {
-		errorMsg := fmt.Errorf(
-			"Removing osd.%s would require a downgrade of the automatic crush rule from 'host' to 'osd' level. "+
-				"Likely this will result in additional data movement. Please confirm by setting the "+
-				"'--confirm-failure-domain-downgrade' flag to true",
-			osd,
-		)
-		return response.BadRequest(errorMsg)
+
+	// if check for crush rule scaledown only if crush change is not prohibited.
+	if !req.ProhibitCrushScaledown {
+		needDowngrade, err := ceph.IsDowngradeNeeded(cs, osdid)
+		if err != nil {
+			return response.InternalError(err)
+		}
+		if needDowngrade && !req.ConfirmDowngrade {
+			errorMsg := fmt.Errorf(
+				"removing osd.%s would require a downgrade of the automatic crush rule from 'host' to 'osd' level. "+
+					"Likely this will result in additional data movement. Please confirm by setting the "+
+					"'--confirm-failure-domain-downgrade' flag to true",
+				osd,
+			)
+			return response.BadRequest(errorMsg)
+		}
 	}
 
 	err = ceph.RemoveOSD(cs, osdid, req.BypassSafety, req.Timeout)

microceph/api/types/disks.go

@@ -29,10 +29,11 @@ type DiskAddResponse struct {
 
 // DisksDelete holds an OSD number and a flag for forcing the removal
 type DisksDelete struct {
-	OSD              int64 `json:"osdid" yaml:"osdid"`
-	BypassSafety     bool  `json:"bypass_safety" yaml:"bypass_safety"`
-	ConfirmDowngrade bool  `json:"confirm_downgrade" yaml:"confirm_downgrade"`
-	Timeout          int64 `json:"timeout" yaml:"timeout"`
+	OSD                    int64 `json:"osdid" yaml:"osdid"`
+	BypassSafety           bool  `json:"bypass_safety" yaml:"bypass_safety"`
+	ConfirmDowngrade       bool  `json:"confirm_downgrade" yaml:"confirm_downgrade"`
+	ProhibitCrushScaledown bool  `json:"prohibit_crush_scaledown" yaml:"prohibit_crush_scaledown"`
+	Timeout                int64 `json:"timeout" yaml:"timeout"`
 }
 
 // Disks is a slice of disks

microceph/cmd/microceph/disk_remove.go

@@ -16,9 +16,10 @@ type cmdDiskRemove struct {
 	common *CmdControl
 	disk   *cmdDisk
 
-	flagBypassSafety     bool
-	flagConfirmDowngrade bool
-	flagTimeout          int64
+	flagBypassSafety           bool
+	flagConfirmDowngrade       bool
+	flagProhibitCrushScaledown bool
+	flagTimeout                int64
 }
 
 func (c *cmdDiskRemove) Command() *cobra.Command {
@@ -31,6 +32,7 @@ func (c *cmdDiskRemove) Command() *cobra.Command {
 	cmd.PersistentFlags().Int64Var(&c.flagTimeout, "timeout", 1800, "Timeout to wait for safe removal (seconds), default=1800")
 	cmd.PersistentFlags().BoolVar(&c.flagBypassSafety, "bypass-safety-checks", false, "Bypass safety checks")
 	cmd.PersistentFlags().BoolVar(&c.flagConfirmDowngrade, "confirm-failure-domain-downgrade", false, "Confirm failure domain downgrade if required")
+	cmd.PersistentFlags().BoolVar(&c.flagProhibitCrushScaledown, "prohibit-crush-scaledown", false, "Remove OSD without scaling down the crush failure domain")
 
 	return cmd
 }
@@ -63,11 +65,16 @@ func (c *cmdDiskRemove) Run(cmd *cobra.Command, args []string) error {
 		}
 	}
 
+	if c.flagConfirmDowngrade && c.flagProhibitCrushScaledown {
+		return fmt.Errorf("bad Request, --confirm-failure-domain-downgrade and --prohibit-crush-scaledown flags are exclusive to each other")
+	}
+
 	req := &types.DisksDelete{
-		OSD:              osd,
-		BypassSafety:     c.flagBypassSafety,
-		ConfirmDowngrade: c.flagConfirmDowngrade,
-		Timeout:          c.flagTimeout,
+		OSD:                    osd,
+		BypassSafety:           c.flagBypassSafety,
+		ConfirmDowngrade:       c.flagConfirmDowngrade,
+		ProhibitCrushScaledown: c.flagProhibitCrushScaledown,
+		Timeout:                c.flagTimeout,
 	}
 
 	fmt.Printf("Removing osd.%d, timeout %ds\n", osd, req.Timeout)

tests/scripts/actionutils.sh

@@ -280,7 +280,7 @@ function cluster_nodes() {
 
 function add_osd_to_node() {
     local container="${1?missing}"
-    lxc exec $container -- sh -c "microceph disk add /dev/sdia"
+    lxc exec $container -- sh -c "microceph disk add /dev/sdia --wipe"
     sleep 1
 }