Repository: Azure/ARO-RP
Pull request #2655 (2023-01-24 05:52:34)

Change az aro create install-version argument to version
### Which issue this PR addresses:

Changes the `az aro create` CLI argument for multi-version from `install-version` to `version`.

### What this PR does / why we need it:

This change was requested from product management to ensure a consistent experience for end-users, because we list the versions as simply `Version` and also the ARM template cluster creation accepts `version`.

### Test plan for issue:

Tested cluster creation and version validation manually.

### Is there any documentation that needs to be updated for this PR?

Yes. The documentation for multi-version should specify `version` and not `install-version`.



Patch:
diff --git a/.pipelines/ci.yml b/.pipelines/ci.yml
index 61a10f7f0..545fe81b5 100644
--- a/.pipelines/ci.yml
+++ b/.pipelines/ci.yml
@@ -33,7 +33,7 @@ variables:
 jobs:
 - job: Python_Unit_Tests
   pool:
-    name: aro-ci-1es
+    name: 1es-aro-ci-pool
   variables:
     HOME: $(Agent.BuildDirectory)
   steps:
@@ -47,7 +47,7 @@ jobs:
 
 - job: Golang_Unit_Tests
   pool:
-    name: aro-ci-1es
+    name: 1es-aro-ci-pool
   variables:
     GOCACHE: /tmp/gocache
   steps:
@@ -110,7 +110,7 @@ jobs:
 
 - job: Lint_Admin_Portal
   pool:
-    name: aro-ci-1es
+    name: 1es-aro-ci-pool
   steps:
   - template: ./templates/template-checkout.yml
   - script: |
diff --git a/pkg/dbtoken/refresher.go b/pkg/dbtoken/refresher.go
index 5648910b4..a17d80d6b 100644
--- a/pkg/dbtoken/refresher.go
+++ b/pkg/dbtoken/refresher.go
@@ -68,7 +68,7 @@ func (r *refresher) checkRefreshAndReset() bool {
 func (r *refresher) Run(ctx context.Context) error {
 	defer utilrecover.Panic(r.log)
 
-	go heartbeat.EmitHeartbeat(r.log, r.m, r.metricPrefix+"DbTokenRefresh", nil, r.checkRefreshAndReset)
+	go heartbeat.EmitHeartbeat(r.log, r.m, r.metricPrefix+".dbtokenrefresh", nil, r.checkRefreshAndReset)
 
 	t := time.NewTicker(10 * time.Second)
 	defer t.Stop()
diff --git a/pkg/util/cluster/cluster.go b/pkg/util/cluster/cluster.go
index 952cfa2ff..1fc57cd69 100644
--- a/pkg/util/cluster/cluster.go
+++ b/pkg/util/cluster/cluster.go
@@ -339,12 +339,15 @@ func (c *Cluster) Create(ctx context.Context, vnetResourceGroup, clusterName str
 
 func (c *Cluster) generateSubnets() (vnetPrefix string, masterSubnet string, workerSubnet string) {
 	// pick a random 23 in range [10.3.0.0, 10.127.255.0]
-	// 10.0.0.0 is used by dev-vnet to host CI
-	// 10.1.0.0 is used by rp-vnet to host Proxy VM
-	// 10.2.0.0 is used by dev-vpn-vnet to host VirtualNetworkGateway
+	// 10.0.0.0/16 is used by dev-vnet to host CI
+	// 10.1.0.0/24 is used by rp-vnet to host Proxy VM
+	// 10.2.0.0/24 is used by dev-vpn-vnet to host VirtualNetworkGateway
 	var x, y int
 	rand.Seed(time.Now().UnixNano())
-	for x == 0 && y == 0 {
+	// Local Dev clusters are limited to /16 dev-vnet
+	if !c.ci {
+		x, y = 0, 2*rand.Intn(128)
+	} else {
 		x, y = rand.Intn((124))+3, 2*rand.Intn(128)
 	}
 	vnetPrefix = fmt.Sprintf("10.%d.%d.0/23", x, y)
diff --git a/python/az/aro/HISTORY.rst b/python/az/aro/HISTORY.rst
index c5a7f9541..0f6e95fd7 100644
--- a/python/az/aro/HISTORY.rst
+++ b/python/az/aro/HISTORY.rst
@@ -54,4 +54,4 @@ Release History
 1.0.7
 ++++++
 * Add support for get-versions (lists available installation versions of OpenShift per location)
-* Adds an aro create 'install-version' parameter (for specifying the cluster installation version)
+* Adds an aro create 'version' parameter (for specifying the cluster installation version)
diff --git a/python/az/aro/azext_aro/_help.py b/python/az/aro/azext_aro/_help.py
index 06047ad78..dde7ed271 100644
--- a/python/az/aro/azext_aro/_help.py
+++ b/python/az/aro/azext_aro/_help.py
@@ -16,7 +16,7 @@ helps['aro create'] = """
   - name: Create a cluster.
     text: az aro create --resource-group MyResourceGroup --name MyCluster --vnet MyVnet --master-subnet MyMasterSubnet --worker-subnet MyWorkerSubnet
   - name: Create a cluster with a supported OpenShift version.
-    text: az aro create --resource-group MyResourceGroup --name MyCluster --vnet MyVnet --master-subnet MyMasterSubnet --worker-subnet MyWorkerSubnet --install-version X.Y.Z
+    text: az aro create --resource-group MyResourceGroup --name MyCluster --vnet MyVnet --master-subnet MyMasterSubnet --worker-subnet MyWorkerSubnet --version X.Y.Z
   - name: Create a cluster with 5 compute nodes and Red Hat pull secret.
     text: az aro create --resource-group MyResourceGroup --name MyCluster --vnet MyVnet --master-subnet MyMasterSubnet --worker-subnet MyWorkerSubnet --worker-count 5 --pull-secret @pullsecret.txt
   - name: Create a private cluster.
diff --git a/python/az/aro/azext_aro/_params.py b/python/az/aro/azext_aro/_params.py
index 955353624..c880d3ea1 100644
--- a/python/az/aro/azext_aro/_params.py
+++ b/python/az/aro/azext_aro/_params.py
@@ -16,7 +16,7 @@ from azext_aro._validators import validate_vnet_resource_group_name
 from azext_aro._validators import validate_worker_count
 from azext_aro._validators import validate_worker_vm_disk_size_gb
 from azext_aro._validators import validate_refresh_cluster_credentials
-from azext_aro._validators import validate_install_version_format
+from azext_aro._validators import validate_version_format
 from azure.cli.core.commands.parameters import name_type
 from azure.cli.core.commands.parameters import get_enum_type, get_three_state_flag
 from azure.cli.core.commands.parameters import resource_group_name_type
@@ -54,9 +54,9 @@ def load_arguments(self, _):
                    help='Client secret of cluster service principal.',
                    validator=validate_client_secret(isCreate=True))
 
-        c.argument('install_version',
+        c.argument('version',
                    help='OpenShift version to use for cluster creation.',
-                   validator=validate_install_version_format)
+                   validator=validate_version_format)
 
         c.argument('pod_cidr',
                    help='CIDR of pod network. Must be a minimum of /18 or larger.',
diff --git a/python/az/aro/azext_aro/_validators.py b/python/az/aro/azext_aro/_validators.py
index c63a182fd..9862ee78a 100644
--- a/python/az/aro/azext_aro/_validators.py
+++ b/python/az/aro/azext_aro/_validators.py
@@ -260,8 +260,6 @@ def validate_refresh_cluster_credentials(namespace):
         raise RequiredArgumentMissingError('--client-id and --client-secret must be not set with --refresh-credentials.')  # pylint: disable=line-too-long
 
 
-def validate_install_version_format(namespace):
-    if namespace.install_version is not None and not re.match(r'^' +
-                                                              r'[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}' +
-                                                              r'$', namespace.install_version):
-        raise InvalidArgumentValueError('--install-version is invalid')
+def validate_version_format(namespace):
+    if namespace.version is not None and not re.match(r'^[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}$', namespace.version):
+        raise InvalidArgumentValueError('--version is invalid')
diff --git a/python/az/aro/azext_aro/custom.py b/python/az/aro/azext_aro/custom.py
index f65230a23..2b91a2a40 100644
--- a/python/az/aro/azext_aro/custom.py
+++ b/python/az/aro/azext_aro/custom.py
@@ -58,7 +58,7 @@ def aro_create(cmd,  # pylint: disable=too-many-locals
                apiserver_visibility=None,
                ingress_visibility=None,
                tags=None,
-               install_version=None,
+               version=None,
                no_wait=False):
     if not rp_mode_development():
         resource_client = get_mgmt_service_client(
@@ -106,7 +106,7 @@ def aro_create(cmd,  # pylint: disable=too-many-locals
             resource_group_id=(f"/subscriptions/{subscription_id}"
                                f"/resourceGroups/{cluster_resource_group or 'aro-' + random_id}"),
             fips_validated_modules='Enabled' if fips_validated_modules else 'Disabled',
-            version=install_version or '',
+            version=version or '',
 
         ),
         service_principal_profile=openshiftcluster.ServicePrincipalProfile(


Test patch:
diff --git a/test/e2e/operator.go b/test/e2e/operator.go
index 25a805ce0..d6e5f947e 100644
--- a/test/e2e/operator.go
+++ b/test/e2e/operator.go
@@ -311,9 +311,14 @@ var _ = Describe("ARO Operator - Azure Subnet Reconciler", func() {
 })
 
 var _ = Describe("ARO Operator - MUO Deployment", func() {
+	const (
+		managedUpgradeOperatorNamespace  = "openshift-managed-upgrade-operator"
+		managedUpgradeOperatorDeployment = "managed-upgrade-operator"
+	)
+
 	It("must be deployed by default with FIPS crypto mandated", func(ctx context.Context) {
 		By("getting MUO pods")
-		pods, err := clients.Kubernetes.CoreV1().Pods("openshift-managed-upgrade-operator").List(ctx, metav1.ListOptions{
+		pods, err := clients.Kubernetes.CoreV1().Pods(managedUpgradeOperatorNamespace).List(ctx, metav1.ListOptions{
 			LabelSelector: "name=managed-upgrade-operator",
 		})
 		Expect(err).NotTo(HaveOccurred())
@@ -321,12 +326,39 @@ var _ = Describe("ARO Operator - MUO Deployment", func() {
 
 		By("verifying that MUO has FIPS crypto mandated by reading logs")
 		Eventually(func(g Gomega, ctx context.Context) {
-			b, err := clients.Kubernetes.CoreV1().Pods("openshift-managed-upgrade-operator").GetLogs(pods.Items[0].Name, &corev1.PodLogOptions{}).DoRaw(ctx)
+			b, err := clients.Kubernetes.CoreV1().Pods(managedUpgradeOperatorNamespace).GetLogs(pods.Items[0].Name, &corev1.PodLogOptions{}).DoRaw(ctx)
 			g.Expect(err).NotTo(HaveOccurred())
 
 			g.Expect(string(b)).To(ContainSubstring(`msg="FIPS crypto mandated: true"`))
 		}).WithContext(ctx).Should(Succeed())
 	})
+
+	It("must be restored if deleted", func(ctx context.Context) {
+		deleteMUODeployment := func(ctx context.Context) error {
+			return clients.Kubernetes.
+				AppsV1().
+				Deployments(managedUpgradeOperatorNamespace).
+				Delete(ctx, managedUpgradeOperatorDeployment, metav1.DeleteOptions{})
+		}
+
+		muoDeploymentExists := func(g Gomega, ctx context.Context) {
+			_, err := clients.Kubernetes.
+				AppsV1().
+				Deployments(managedUpgradeOperatorNamespace).
+				Get(ctx, managedUpgradeOperatorDeployment, metav1.GetOptions{})
+
+			g.Expect(err).ToNot(HaveOccurred())
+		}
+
+		By("waiting for the MUO deployment to be ready")
+		Eventually(muoDeploymentExists).WithContext(ctx).Should(Succeed())
+
+		By("deleting the MUO deployment")
+		Expect(deleteMUODeployment(ctx)).Should(Succeed())
+
+		By("waiting for the MUO deployment to be reconciled")
+		Eventually(muoDeploymentExists).WithContext(ctx).Should(Succeed())
+	})
 })
 
 var _ = Describe("ARO Operator - ImageConfig Reconciler", func() {