Merge pull request #107 from anirudhAgniRedhat/anirudh/capacityReservationGroups

CFE-1050: Add support of capacity reservation group in openshift/machine-api-provider-azure

Author: openshift-merge-bot[bot]

.gitignore

@@ -45,3 +45,6 @@ bin/
 
 # vscode
 .vscode
+
+# idea
+.idea

Makefile

@@ -27,7 +27,7 @@ LD_FLAGS    ?= -X $(REPO_PATH)/pkg/version.Raw=$(VERSION) -extldflags -static
 BUILD_IMAGE ?= registry.ci.openshift.org/openshift/release:golang-1.21
 
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.29
+ENVTEST_K8S_VERSION = 1.29.1
 
 PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
 CONTROLLER_GEN = go run ${PROJECT_DIR}/vendor/sigs.k8s.io/controller-tools/cmd/controller-gen
@@ -115,7 +115,7 @@ build: ## build binaries
 .PHONY: test
 test: ## Run tests
 	@echo -e "\033[32mTesting...\033[0m"
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path --bin-dir $(PROJECT_DIR)/bin)" ./hack/ci-test.sh
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path --bin-dir $(PROJECT_DIR)/bin --remote-bucket openshift-kubebuilder-tools)" ./hack/ci-test.sh
 
 .PHONY: unit
 unit: # Run unit test

cmd/manager/main.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	configv1 "github.com/openshift/api/config/v1"
+	apifeatures "github.com/openshift/api/features"
 	machinev1 "github.com/openshift/api/machine/v1beta1"
 	configclient "github.com/openshift/client-go/config/clientset/versioned"
 	configinformers "github.com/openshift/client-go/config/informers/externalversions"
@@ -161,7 +162,7 @@ func main() {
 		klog.Fatalf("Failed to get feature gates: %v", err)
 	}
 
-	azureWorkloadIdentityEnabled := featureGates.Enabled(configv1.FeatureGateAzureWorkloadIdentity)
+	azureWorkloadIdentityEnabled := featureGates.Enabled(apifeatures.FeatureGateAzureWorkloadIdentity)
 
 	// Initialize machine actuator.
 	machineActuator := actuator.NewActuator(actuator.ActuatorParams{

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/onsi/ginkgo/v2 v2.14.0
 	github.com/onsi/gomega v1.30.0
-	github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e
+	github.com/openshift/api v0.0.0-20240521141249-8af21b7ed3e3
 	github.com/openshift/machine-api-operator v0.2.1-0.20240125175440-c9de8bda0dd1
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/cobra v1.8.0

go.sum

@@ -300,8 +300,8 @@ github.com/onsi/ginkgo/v2 v2.14.0 h1:vSmGj2Z5YPb9JwCWT6z6ihcUvDhuXLc3sJiqd3jMKAY
 github.com/onsi/ginkgo/v2 v2.14.0/go.mod h1:JkUdW7JkN0V6rFvsHcJ478egV3XH9NxpD27Hal/PhZw=
 github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8=
 github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ=
-github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e h1:cxgCNo/R769CO23AK5TCh45H9SMUGZ8RukiF2/Qif3o=
-github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
+github.com/openshift/api v0.0.0-20240521141249-8af21b7ed3e3 h1:fxqjG8C/fU1UfUalZhNB01jqIQDlBsCVsFnWZ1V17Rg=
+github.com/openshift/api v0.0.0-20240521141249-8af21b7ed3e3/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
 github.com/openshift/client-go v0.0.0-20240115204758-e6bf7d631d5e h1:qGjfKX8i0h4efMNEnhgTdxcdx6gwwOwhTfBJ20WFqA8=
 github.com/openshift/client-go v0.0.0-20240115204758-e6bf7d631d5e/go.mod h1:2am3qrggh9LlDCf/MDGzcFWMhdaushxFQi0+ZZDhdVk=
 github.com/openshift/library-go v0.0.0-20240116081341-964bcb3f545c h1:gLylEQQryG+A6nqWYIwE1wUzn1eFUmthjADvflMWKnM=

pkg/cloud/azure/actuators/machine/reconciler.go

@@ -47,7 +47,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
-
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -67,7 +66,13 @@ const (
 	// MachineInstanceTypeLabelName as annotation name for a machine instance type
 	MachineInstanceTypeLabelName = "machine.openshift.io/instance-type"
 
-	MachineSetLabelName = "machine.openshift.io/cluster-api-machineset"
+	MachineSetLabelName           = "machine.openshift.io/cluster-api-machineset"
+	azureProviderIDPrefix         = "azure://"
+	azureProvidersKey             = "providers"
+	azureSubscriptionsKey         = "subscriptions"
+	azureResourceGroupsLowerKey   = "resourcegroups"
+	azureLocationsKey             = "locations"
+	azureBuiltInResourceNamespace = "Microsoft.Resources"
 )
 
 // Reconciler are list of services required by cluster actuator, easy to create a fake
@@ -681,6 +686,13 @@ func (s *Reconciler) createVirtualMachine(ctx context.Context, nicName, asName s
 			vmSpec.ManagedIdentity = azure.GenerateManagedIdentityName(s.scope.SubscriptionID, s.scope.MachineConfig.ResourceGroup, s.scope.MachineConfig.ManagedIdentity)
 		}
 
+		if s.scope.MachineConfig.CapacityReservationGroupID != "" {
+			if err = validateAzureCapacityReservationGroupID(s.scope.MachineConfig.CapacityReservationGroupID); err != nil {
+				return fmt.Errorf("failed to validate capacityReservationGroupID: %w", err)
+			}
+			vmSpec.CapacityReservationGroupID = s.scope.MachineConfig.CapacityReservationGroupID
+		}
+
 		userData, userDataErr := s.getCustomUserData()
 		if userDataErr != nil {
 			return fmt.Errorf("failed to get custom script data: %w", userDataErr)
@@ -857,3 +869,65 @@ func createDiagnosticsConfig(config *machinev1.AzureMachineProviderSpec) (*compu
 		)
 	}
 }
+
+func validateAzureCapacityReservationGroupID(capacityReservationGroupID string) error {
+	id := strings.TrimPrefix(capacityReservationGroupID, azureProviderIDPrefix)
+	err := parseAzureResourceID(id)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+// parseAzureResourceID parses a string to an instance of ResourceID
+func parseAzureResourceID(id string) error {
+	if len(id) == 0 {
+		return fmt.Errorf("invalid resource ID: id cannot be empty")
+	}
+	if !strings.HasPrefix(id, "/") {
+		return fmt.Errorf("invalid resource ID: resource id '%s' must start with '/'", id)
+	}
+	parts := splitStringAndOmitEmpty(id, "/")
+
+	if len(parts) < 2 {
+		return fmt.Errorf("invalid resource ID: %s", id)
+	}
+	if !strings.EqualFold(parts[0], azureSubscriptionsKey) && !strings.EqualFold(parts[0], azureProvidersKey) {
+		return fmt.Errorf("invalid resource ID: %s", id)
+	}
+	return appendNextAzureResourceIDValidation(parts, id)
+}
+
+func splitStringAndOmitEmpty(v, sep string) []string {
+	r := make([]string, 0)
+	for _, s := range strings.Split(v, sep) {
+		if len(s) == 0 {
+			continue
+		}
+		r = append(r, s)
+	}
+	return r
+}
+
+func appendNextAzureResourceIDValidation(parts []string, id string) error {
+	if len(parts) == 0 {
+		return nil
+	}
+	if len(parts) == 1 {
+		// subscriptions and resourceGroups are not valid ids without their names
+		if strings.EqualFold(parts[0], azureSubscriptionsKey) || strings.EqualFold(parts[0], azureResourceGroupsLowerKey) {
+			return fmt.Errorf("invalid resource ID: %s", id)
+		}
+		return nil
+	}
+	if strings.EqualFold(parts[0], azureProvidersKey) && (len(parts) == 2 || strings.EqualFold(parts[2], azureProvidersKey)) {
+		return appendNextAzureResourceIDValidation(parts[2:], id)
+	}
+	if len(parts) > 3 && strings.EqualFold(parts[0], azureProvidersKey) {
+		return appendNextAzureResourceIDValidation(parts[4:], id)
+	}
+	if len(parts) > 1 && !strings.EqualFold(parts[0], azureProvidersKey) {
+		return appendNextAzureResourceIDValidation(parts[2:], id)
+	}
+	return fmt.Errorf("invalid resource ID: %s", id)
+}

pkg/cloud/azure/actuators/machine/reconciler_test.go

@@ -620,3 +620,44 @@ func TestCreateDiagnosticsConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateCapacityReservationGroupID(t *testing.T) {
+	testCases := []struct {
+		name          string
+		inputID       string
+		expectedError error
+	}{
+		{
+			name:          "validation for capacityReservationGroupID should return nil error if field input is valid",
+			inputID:       "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup",
+			expectedError: nil,
+		},
+		{
+			name:          "validation for capacityReservationGroupID should return error if field input does not start with '/'",
+			inputID:       "subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup",
+			expectedError: errors.New("invalid resource ID: resource id 'subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup' must start with '/'"),
+		},
+		{
+			name:          "validation for capacityReservationGroupID should return error if field input does not have field name subscriptions",
+			inputID:       "/subscripti/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup",
+			expectedError: errors.New("invalid resource ID: /subscripti/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup"),
+		},
+		{
+			name:          "validation for capacityReservationGroupID should return error if field input is empty",
+			inputID:       "",
+			expectedError: errors.New("invalid resource ID: id cannot be empty"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+			err := validateAzureCapacityReservationGroupID(tc.inputID)
+			if tc.expectedError != nil {
+				g.Expect(err).To(MatchError(tc.expectedError))
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
+			}
+		})
+	}
+}

pkg/cloud/azure/actuators/machineset/controller_suite_test.go

@@ -57,8 +57,8 @@ func TestReconciler(t *testing.T) {
 var _ = BeforeSuite(func() {
 	testEnv = &envtest.Environment{
 		CRDDirectoryPaths: []string{
-			filepath.Join("..", "..", "..", "..", "..", "vendor", "github.com", "openshift", "api", "machine", "v1beta1"),
-			filepath.Join("..", "..", "..", "..", "..", "vendor", "github.com", "openshift", "api", "config", "v1"),
+			filepath.Join("..", "..", "..", "..", "..", "vendor", "github.com", "openshift", "api", "machine", "v1beta1", "zz_generated.crd-manifests"),
+			filepath.Join("..", "..", "..", "..", "..", "vendor", "github.com", "openshift", "api", "config", "v1", "zz_generated.crd-manifests"),
 		},
 	}
 

pkg/cloud/azure/services/virtualmachines/virtualmachines.go

@@ -69,24 +69,25 @@ const (
 
 // Spec input specification for Get/CreateOrUpdate/Delete calls
 type Spec struct {
-	Name                string
-	NICName             string
-	SSHKeyData          string
-	Size                string
-	Zone                string
-	Image               machinev1.Image
-	OSDisk              machinev1.OSDisk
-	DataDisks           []machinev1.DataDisk
-	CustomData          string
-	ManagedIdentity     string
-	Tags                map[string]*string
-	Priority            compute.VirtualMachinePriorityTypes
-	EvictionPolicy      compute.VirtualMachineEvictionPolicyTypes
-	BillingProfile      *compute.BillingProfile
-	SecurityProfile     *machinev1.SecurityProfile
-	DiagnosticsProfile  *compute.DiagnosticsProfile
-	UltraSSDCapability  machinev1.AzureUltraSSDCapabilityState
-	AvailabilitySetName string
+	Name                       string
+	NICName                    string
+	SSHKeyData                 string
+	Size                       string
+	Zone                       string
+	Image                      machinev1.Image
+	OSDisk                     machinev1.OSDisk
+	DataDisks                  []machinev1.DataDisk
+	CustomData                 string
+	ManagedIdentity            string
+	Tags                       map[string]*string
+	Priority                   compute.VirtualMachinePriorityTypes
+	EvictionPolicy             compute.VirtualMachineEvictionPolicyTypes
+	BillingProfile             *compute.BillingProfile
+	SecurityProfile            *machinev1.SecurityProfile
+	DiagnosticsProfile         *compute.DiagnosticsProfile
+	UltraSSDCapability         machinev1.AzureUltraSSDCapabilityState
+	AvailabilitySetName        string
+	CapacityReservationGroupID string
 }
 
 // Get provides information about a virtual network.
@@ -336,6 +337,15 @@ func (s *Service) deriveVirtualMachineParameters(vmSpec *Spec, nic network.Inter
 		}
 	}
 
+	// configure capacity reservation ID
+	if vmSpec.CapacityReservationGroupID != "" {
+		virtualMachine.VirtualMachineProperties.CapacityReservation = &compute.CapacityReservationProfile{
+			CapacityReservationGroup: &compute.SubResource{
+				ID: &vmSpec.CapacityReservationGroupID,
+			},
+		}
+	}
+
 	return virtualMachine, nil
 }
 

pkg/cloud/azure/services/virtualmachines/virtualmachines_test.go

@@ -713,6 +713,28 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 			},
 			expectedError: nil,
 		},
+		{
+			name: "capacity reservation group ID should be configured if the string is non empty",
+			updateSpec: func(vmSpec *Spec) {
+				vmSpec.Name = "testvm"
+				vmSpec.CapacityReservationGroupID = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup"
+			},
+			validate: func(g *WithT, vm *compute.VirtualMachine) {
+				g.Expect(*vm.VirtualMachineProperties.CapacityReservation.CapacityReservationGroup.ID).Should(Equal("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup"))
+			},
+			expectedError: nil,
+		},
+		{
+			name: "capacity reservation group ID should be nil if the string is empty",
+			updateSpec: func(vmSpec *Spec) {
+				vmSpec.Name = "testvm"
+				vmSpec.CapacityReservationGroupID = ""
+			},
+			validate: func(g *WithT, vm *compute.VirtualMachine) {
+				g.Expect(vm.VirtualMachineProperties.CapacityReservation).To(BeNil())
+			},
+			expectedError: nil,
+		},
 	}
 
 	for _, tc := range testCases {