Repository: Azure/ARO-RP
Pull request #1977 (2022-02-23 11:05:07)

fix: use the tag/commit as the aro version and copy image to the integration
### Which issue this PR addresses:

Fixes https://msazure.visualstudio.com/AzureRedHatOpenShift/_workitems/edit/13493157
 
### What this PR does / why we need it:

ARO uses both tags and commits as its version.
The commits are used for the development scenario, tags are used when building and deploying to production.

Adds the pipeline step to copy the resulted image to the integration registry.

### Test plan for issue:
N/A

### Is there any documentation that needs to be updated for this PR?

Part of the pipeline documentation.



Patch:
diff --git a/.pipelines/prod-release-tag.yml b/.pipelines/prod-release-tag.yml
index bb9f2baf6..0e36b9e6b 100644
--- a/.pipelines/prod-release-tag.yml
+++ b/.pipelines/prod-release-tag.yml
@@ -80,6 +80,7 @@ stages:
       - southcentralus
       - westus
       - westus2
+      - westus3
       configFileName: prod-config.yaml
       azureDevOpsJSONSPN: $(aro-v4-ci-devops-spn)
       vsoProjectID: $(vso-project-id)
diff --git a/.pipelines/prod-release.yml b/.pipelines/prod-release.yml
index e1aacd527..687f815e2 100644
--- a/.pipelines/prod-release.yml
+++ b/.pipelines/prod-release.yml
@@ -76,6 +76,7 @@ stages:
       - southcentralus
       - westus
       - westus2
+      - westus3
       configFileName: prod-config.yaml
       configVersion: ${{ parameters.configVersion }}
       azureDevOpsJSONSPN: $(aro-v4-ci-devops-spn)
diff --git a/.pipelines/templates/template-push-images-to-acr-tagged.yml b/.pipelines/templates/template-push-images-to-acr-tagged.yml
index 018aa4521..75a689a31 100644
--- a/.pipelines/templates/template-push-images-to-acr-tagged.yml
+++ b/.pipelines/templates/template-push-images-to-acr-tagged.yml
@@ -10,5 +10,11 @@ steps:
     az acr login --name "$RP_IMAGE_ACR"
     # azure checkouts commit, so removing master reference when publishing image
     export BRANCH=$(Build.SourceBranchName)
-    make publish-image-aro-tag
+    make publish-image-aro
   displayName: ⚙️ Build and push tagged images to ACR
+- script: |
+    az acr login --name "arointsvc"
+    az acr import \
+      --name "arointsvc" \
+      --source "${{parameters.rpImageACR}}.azurecr.io/aro:${{parameters.imageTag}}"
+  displayName: ➜ Copy ARO image to the integration registry
diff --git a/Makefile b/Makefile
index eca40f6df..e57f07af2 100644
--- a/Makefile
+++ b/Makefile
@@ -2,7 +2,6 @@ SHELL = /bin/bash
 TAG ?= $(shell git describe --exact-match 2>/dev/null)
 COMMIT = $(shell git rev-parse --short=7 HEAD)$(shell [[ $$(git status --porcelain) = "" ]] || echo -dirty)
 ARO_IMAGE_BASE = ${RP_IMAGE_ACR}.azurecr.io/aro
-ARO_IMAGE ?= $(ARO_IMAGE_BASE):$(COMMIT)
 
 # fluentbit version must also be updated in RP code, see pkg/util/version/const.go
 FLUENTBIT_VERSION = 1.7.8-1
@@ -14,14 +13,22 @@ ifneq ($(shell uname -s),Darwin)
     export CGO_CFLAGS=-Dgpgme_off_t=off_t
 endif
 
+ifeq ($(TAG),)
+	VERSION = $(COMMIT)
+else
+	VERSION = $(TAG)
+endif
+
+ARO_IMAGE ?= $(ARO_IMAGE_BASE):$(VERSION)
+
 build-all:
 	go build -tags aro,containers_image_openpgp ./...
 
 aro: generate
-	go build -tags aro,containers_image_openpgp,codec.safe -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(COMMIT)" ./cmd/aro
+	go build -tags aro,containers_image_openpgp,codec.safe -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(VERSION)" ./cmd/aro
 
 runlocal-rp:
-	go run -tags aro -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(COMMIT)" ./cmd/aro rp
+	go run -tags aro -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(VERSION)" ./cmd/aro rp
 
 az: pyenv
 	. pyenv/bin/activate && \
@@ -42,7 +49,7 @@ client: generate
 # TODO: hard coding dev-config.yaml is clunky; it is also probably convenient to
 # override COMMIT.
 deploy:
-	go run -tags aro -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(COMMIT)" ./cmd/aro deploy dev-config.yaml ${LOCATION}
+	go run -tags aro -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(VERSION)" ./cmd/aro deploy dev-config.yaml ${LOCATION}
 
 dev-config.yaml:
 	go run ./hack/gendevconfig >dev-config.yaml
@@ -59,12 +66,6 @@ image-aro: aro e2e.test
 	docker pull registry.access.redhat.com/ubi8/ubi-minimal
 	docker build --network=host --no-cache -f Dockerfile.aro -t $(ARO_IMAGE) .
 
-image-aro-tag: image-aro
-ifeq ($(TAG),)
-	$(error TAG undefined)
-endif
-	docker tag $(ARO_IMAGE) $(ARO_IMAGE_BASE):$(TAG)
-
 image-aro-multistage:
 	docker build --network=host --no-cache -f Dockerfile.aro-multistage -t $(ARO_IMAGE) .
 
@@ -87,12 +88,6 @@ ifeq ("${RP_IMAGE_ACR}-$(BRANCH)","arointsvc-master")
 		docker push arointsvc.azurecr.io/aro:latest
 endif
 
-publish-image-aro-tag: image-aro-tag
-ifeq ($(TAG),)
-	$(error TAG undefined)
-endif
-	docker push $(ARO_IMAGE_BASE):$(TAG)
-
 publish-image-aro-multistage: image-aro-multistage
 	docker push $(ARO_IMAGE)
 ifeq ("${RP_IMAGE_ACR}-$(BRANCH)","arointsvc-master")
@@ -110,10 +105,10 @@ publish-image-proxy: image-proxy
 	docker push ${RP_IMAGE_ACR}.azurecr.io/proxy:latest
 
 proxy:
-	go build -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(COMMIT)" ./hack/proxy
+	go build -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(VERSION)" ./hack/proxy
 
 run-portal:
-	go run -tags aro -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(COMMIT)" ./cmd/aro portal
+	go run -tags aro -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(VERSION)" ./cmd/aro portal
 
 build-portal:
 	cd portal && npm install && npm run build
@@ -143,7 +138,7 @@ tunnel:
 	go run ./hack/tunnel $(shell az network public-ip show -g ${RESOURCEGROUP} -n rp-pip --query 'ipAddress')
 
 e2e.test:
-	go test ./test/e2e -tags e2e,codec.safe -c -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(COMMIT)" -o e2e.test
+	go test ./test/e2e -tags e2e,codec.safe -c -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(VERSION)" -o e2e.test
 
 test-e2e: e2e.test
 	./e2e.test -test.timeout 180m -test.v -ginkgo.v
diff --git a/pkg/frontend/openshiftcluster_putorpatch.go b/pkg/frontend/openshiftcluster_putorpatch.go
index 087770e6d..19fece5b4 100644
--- a/pkg/frontend/openshiftcluster_putorpatch.go
+++ b/pkg/frontend/openshiftcluster_putorpatch.go
@@ -84,10 +84,9 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus.
 				},
 			},
 		}
-		// TODO (BV): reenable gateway on create once we fix bugs
-		// if !f.env.IsLocalDevelopmentMode() /* not local dev or CI */ {
-		// 	doc.OpenShiftCluster.Properties.FeatureProfile.GatewayEnabled = true
-		// }
+		if !f.env.IsLocalDevelopmentMode() /* not local dev or CI */ {
+			doc.OpenShiftCluster.Properties.FeatureProfile.GatewayEnabled = true
+		}
 	}
 
 	doc.CorrelationData = correlationData
diff --git a/pkg/operator/deploy/deploy.go b/pkg/operator/deploy/deploy.go
index 7b08ba15d..fae77da2c 100644
--- a/pkg/operator/deploy/deploy.go
+++ b/pkg/operator/deploy/deploy.go
@@ -175,10 +175,12 @@ func (o *operator) resources() ([]kruntime.Object, error) {
 		},
 	}
 
-	// TODO (BV): reenable gateway once we fix bugs
-	// if o.oc.Properties.NetworkProfile.GatewayPrivateEndpointIP != "" {
-	// 	cluster.Spec.GatewayDomains = append(o.env.GatewayDomains(), o.oc.Properties.ImageRegistryStorageAccountName+".blob."+o.env.Environment().StorageEndpointSuffix)
-	// }
+	if o.oc.Properties.FeatureProfile.GatewayEnabled && o.oc.Properties.NetworkProfile.GatewayPrivateEndpointIP != "" {
+		cluster.Spec.GatewayDomains = append(o.env.GatewayDomains(), o.oc.Properties.ImageRegistryStorageAccountName+".blob."+o.env.Environment().StorageEndpointSuffix)
+	} else {
+		// covers the case of an admin-disable, we need to update dnsmasq on each node
+		cluster.Spec.GatewayDomains = make([]string, 0)
+	}
 
 	// create a secret here for genevalogging, later we will copy it to
 	// the genevalogging namespace.


Test patch:
diff --git a/.pipelines/e2e-with-billing-all-regions.yml b/.pipelines/e2e-with-billing-all-regions.yml
index d6cd44400..14dfde24b 100644
--- a/.pipelines/e2e-with-billing-all-regions.yml
+++ b/.pipelines/e2e-with-billing-all-regions.yml
@@ -68,6 +68,7 @@ stages:
     - southcentralus
     - westus
     - westus2
+    - westus3
 - stage: PhaseThree
   displayName: wait 2 hours to avoid resource threshold
   dependsOn: []
diff --git a/pkg/api/admin/openshiftcluster_validatestatic.go b/pkg/api/admin/openshiftcluster_validatestatic.go
index f94c6a338..bb56358e4 100644
--- a/pkg/api/admin/openshiftcluster_validatestatic.go
+++ b/pkg/api/admin/openshiftcluster_validatestatic.go
@@ -33,9 +33,5 @@ func (sv *openShiftClusterStaticValidator) validateDelta(oc, current *OpenShiftC
 		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.maintenanceTask", "Invalid enum parameter.")
 	}
 
-	if current.Properties.FeatureProfile.GatewayEnabled && !oc.Properties.FeatureProfile.GatewayEnabled {
-		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, "properties.featureProfile.gatewayEnabled", "Changing property 'properties.featureProfile.gatewayEnabled' is not allowed.")
-	}
-
 	return nil
 }
diff --git a/pkg/api/admin/openshiftcluster_validatestatic_test.go b/pkg/api/admin/openshiftcluster_validatestatic_test.go
index a04448654..a00ca327f 100644
--- a/pkg/api/admin/openshiftcluster_validatestatic_test.go
+++ b/pkg/api/admin/openshiftcluster_validatestatic_test.go
@@ -129,29 +129,34 @@ func TestOpenShiftClusterStaticValidateDelta(t *testing.T) {
 			wantErr: "400: PropertyChangeNotAllowed: properties.lastAdminUpdateError: Changing property 'properties.lastAdminUpdateError' is not allowed.",
 		},
 		{
-			name: "valid gatewayEnabled change",
+			name: "disable gatewayEnabled on enabled clusters is allowed",
 			oc: func() *OpenShiftCluster {
-				return &OpenShiftCluster{}
+				return &OpenShiftCluster{
+					Properties: OpenShiftClusterProperties{
+						FeatureProfile: FeatureProfile{
+							GatewayEnabled: true,
+						},
+					},
+				}
 			},
 			modify: func(oc *OpenShiftCluster) {
-				oc.Properties.FeatureProfile.GatewayEnabled = true
+				oc.Properties.FeatureProfile.GatewayEnabled = false
 			},
 		},
 		{
-			name: "valid gatewayEnabled change",
+			name: "enable gatewayEnabled on disabled clusters is allowed",
 			oc: func() *OpenShiftCluster {
 				return &OpenShiftCluster{
 					Properties: OpenShiftClusterProperties{
 						FeatureProfile: FeatureProfile{
-							GatewayEnabled: true,
+							GatewayEnabled: false,
 						},
 					},
 				}
 			},
 			modify: func(oc *OpenShiftCluster) {
-				oc.Properties.FeatureProfile.GatewayEnabled = false
+				oc.Properties.FeatureProfile.GatewayEnabled = true
 			},
-			wantErr: "400: PropertyChangeNotAllowed: properties.featureProfile.gatewayEnabled: Changing property 'properties.featureProfile.gatewayEnabled' is not allowed.",
 		},
 		{
 			name: "console url change is not allowed",
diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go
index 6f18785df..adeaefd23 100644
--- a/pkg/frontend/openshiftcluster_putorpatch_test.go
+++ b/pkg/frontend/openshiftcluster_putorpatch_test.go
@@ -719,7 +719,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 								EncryptionAtHost: api.EncryptionAtHostDisabled,
 							},
 							FeatureProfile: api.FeatureProfile{
-								GatewayEnabled: false,
+								GatewayEnabled: true,
 							},
 							OperatorFlags: api.DefaultOperatorFlags.Copy(),
 						},
diff --git a/test/e2e/adminapi_redeployvm.go b/test/e2e/adminapi_redeployvm.go
index d9d7762b7..df35a2f24 100644
--- a/test/e2e/adminapi_redeployvm.go
+++ b/test/e2e/adminapi_redeployvm.go
@@ -36,6 +36,7 @@ var _ = Describe("[Admin API] VM redeploy action", func() {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(vms).NotTo(HaveLen(0))
 		vm := vms[0]
+		log.Infof("selected vm: %s", *vm.Name)
 
 		By("triggering the redeploy action")
 		clockDrift := -1 * time.Minute
@@ -67,19 +68,18 @@ var _ = Describe("[Admin API] VM redeploy action", func() {
 			var nodeNotReady, rebooted, nodeReady bool
 
 			for _, event := range events.Items {
-				if event.CreationTimestamp.After(nodeKillTime.Time) {
-					if !nodeNotReady &&
-						event.Reason == "NodeNotReady" &&
-						event.Regarding.Name == *vm.Name {
+				if event.CreationTimestamp.After(nodeKillTime.Time) && event.Regarding.Name == *vm.Name {
+					if !nodeNotReady && event.Reason == "NodeNotReady" {
 						nodeNotReady = true
-					} else if !rebooted &&
-						event.Reason == "Rebooted" &&
-						event.Regarding.Name == *vm.Name {
+						log.Info("node entered not ready state")
+					}
+					if !rebooted && event.Reason == "Rebooted" {
 						rebooted = true
-					} else if !nodeReady &&
-						event.Reason == "NodeReady" &&
-						event.Regarding.Name == *vm.Name {
+						log.Info("node reboooted")
+					}
+					if !nodeReady && event.Reason == "NodeReady" {
 						nodeReady = true
+						log.Info("node returned to ready state")
 						break
 					}
 				}
diff --git a/test/e2e/cluster.go b/test/e2e/cluster.go
index 2bcf0a9c0..1ea23478e 100644
--- a/test/e2e/cluster.go
+++ b/test/e2e/cluster.go
@@ -83,6 +83,18 @@ var _ = Describe("Cluster smoke test", func() {
 			return ready.ServiceIsReady(svc)
 		}, 5*time.Minute, 10*time.Second).Should(BeTrue())
 	})
+
+	// mainly we want to test the gateway/egress functionality - this request for the image will travel from
+	// node > gateway > storage account of the registry.
+	Specify("Can access and use the internal container registry", func() {
+		ctx := context.Background()
+		deployName := "internal-registry-deploy"
+		err := createContainerFromInternalContainerRegistryImage(ctx, clients.Kubernetes, deployName)
+		Expect(err).NotTo(HaveOccurred())
+
+		err = wait.PollImmediate(10*time.Second, 5*time.Minute, ready.CheckDeploymentIsReady(ctx, clients.Kubernetes.AppsV1().Deployments(testNamespace), deployName))
+		Expect(err).NotTo(HaveOccurred())
+	})
 })
 
 func createStatefulSet(ctx context.Context, cli kubernetes.Interface) error {
@@ -167,3 +179,40 @@ func createLoadBalancerService(ctx context.Context, cli kubernetes.Interface, na
 	_, err := cli.CoreV1().Services(testNamespace).Create(context.Background(), svc, metav1.CreateOptions{})
 	return err
 }
+
+func createContainerFromInternalContainerRegistryImage(ctx context.Context, cli kubernetes.Interface, name string) error {
+	deploy := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: testNamespace,
+		},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: to.Int32Ptr(1),
+			Selector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"app": name,
+				},
+			},
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{"app": name},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name:  "cli",
+							Image: "image-registry.openshift-image-registry.svc:5000/openshift/cli",
+							Command: []string{
+								"/bin/sh",
+								"-c",
+								"while true; do sleep 1; done",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	_, err := cli.AppsV1().Deployments(testNamespace).Create(context.Background(), deploy, metav1.CreateOptions{})
+	return err
+}