Reconcile ImageStream only if the FG is true (kubevirt#1659)

If the enableCommonBootImageImport FG is set, create the imageStreams and reconcile them
If the enableCommonBootImageImport FG is not set, remove the imageStreams

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

deploy/cluster_role.yaml

@@ -601,6 +601,7 @@ rules:
   - watch
   - update
   - create
+  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole

deploy/index-image/community-kubevirt-hyperconverged/1.6.0/manifests/kubevirt-hyperconverged-operator.v1.6.0.clusterserviceversion.yaml

@@ -431,6 +431,7 @@ spec:
           - watch
           - update
           - create
+          - delete
         serviceAccountName: hyperconverged-cluster-operator
       - rules:
         - apiGroups:

deploy/olm-catalog/community-kubevirt-hyperconverged/1.6.0/manifests/kubevirt-hyperconverged-operator.v1.6.0.clusterserviceversion.yaml

@@ -9,7 +9,7 @@ metadata:
     categories: OpenShift Optional
     certified: "false"
     containerImage: quay.io/kubevirt/hyperconverged-cluster-operator:1.6.0-unstable
-    createdAt: "2021-12-13 08:52:28"
+    createdAt: "2021-12-16 07:25:42"
     description: A unified operator deploying and controlling KubeVirt and its supporting
       operators with opinionated defaults
     operatorframework.io/initialization-resource: '{"apiVersion":"hco.kubevirt.io/v1beta1","kind":"HyperConverged","metadata":{"annotations":{"deployOVS":"false"},"name":"kubevirt-hyperconverged","namespace":"kubevirt-hyperconverged"},"spec":{}}'
@@ -431,6 +431,7 @@ spec:
           - watch
           - update
           - create
+          - delete
         serviceAccountName: hyperconverged-cluster-operator
       - rules:
         - apiGroups:

hack/check_golden_images.sh

@@ -5,6 +5,12 @@ set -ex
 IMAGES_NS=${IMAGES_NS:-kubevirt-os-images}
 
 if [[ $(${KUBECTL_BINARY} get ssp -n ${INSTALLED_NAMESPACE}) ]]; then
+  [[ $(${KUBECTL_BINARY} get imageStream centos8  -n ${IMAGES_NS} --no-headers | wc -l) -eq 0 ]]
+
+  # Test DataImportCronTemplates
+  ./hack/retry.sh 10 3 "${KUBECTL_BINARY} patch hco -n \"${INSTALLED_NAMESPACE}\" --type=json kubevirt-hyperconverged -p '[{ \"op\": \"replace\", \"path\": \"/spec/featureGates/enableCommonBootImageImport\", \"value\": true }]'"
+  sleep 10
+
   # Test image streams
   [[ $(${KUBECTL_BINARY} get imageStream centos8  -n ${IMAGES_NS} --no-headers | wc -l) -eq 1 ]]
   [[ "$(${KUBECTL_BINARY} get imageStream centos8  -n ${IMAGES_NS} -o json | jq -cM '.spec.tags[0].from')" == '{"kind":"DockerImage","name":"quay.io/kubevirt/centos8-container-disk-images"}' ]]
@@ -15,9 +21,6 @@ if [[ $(${KUBECTL_BINARY} get ssp -n ${INSTALLED_NAMESPACE}) ]]; then
   # HCO expect to remove the test-label label from the image stream
   ./hack/retry.sh 10 3 "[[ -z '$(${KUBECTL_BINARY} get imageStream -n ${IMAGES_NS} centos8 -o jsonpath='{.metadata.labels.test-label}')' ]]" "${KUBECTL_BINARY} get imageStream -n ${IMAGES_NS} centos8 -o yaml"
 
-  # Test DataImportCronTemplates
-  ./hack/retry.sh 10 3 "${KUBECTL_BINARY} patch hco -n \"${INSTALLED_NAMESPACE}\" --type=json kubevirt-hyperconverged -p '[{ \"op\": \"replace\", \"path\": \"/spec/featureGates/enableCommonBootImageImport\", \"value\": true }]'"
-  sleep 10
 
   ${KUBECTL_BINARY} get hco -n "${INSTALLED_NAMESPACE}" kubevirt-hyperconverged -o jsonpath='{.spec.featureGates.enableCommonBootImageImport}'
   ${KUBECTL_BINARY} get ssp -n "${INSTALLED_NAMESPACE}" ssp-kubevirt-hyperconverged -o jsonpath='{.spec.commonTemplates.dataImportCronTemplates}' | jq -e '.[] |select(.metadata.name=="centos8-image-cron")'
@@ -35,5 +38,7 @@ if [[ $(${KUBECTL_BINARY} get ssp -n ${INSTALLED_NAMESPACE}) ]]; then
   ./hack/retry.sh 10 3 "${KUBECTL_BINARY} patch hco -n \"${INSTALLED_NAMESPACE}\" --type=json kubevirt-hyperconverged -p '[{ \"op\": \"replace\", \"path\": \"/spec/featureGates/enableCommonBootImageImport\", \"value\": false }]'"
   sleep 10
 
+  #check that the image streams and the DataImportCron were removed
+  ./hack/retry.sh 10 3 "[[ $(${KUBECTL_BINARY} get imageStream centos8  -n ${IMAGES_NS} --no-headers | wc -l) -eq 0 ]]"
   ./hack/retry.sh 10 3 "[[ $(${KUBECTL_BINARY} get DataImportCron -A --no-headers | wc -l) -eq 0 ]]"
 fi

hack/upgrade-test-index-image.sh

@@ -309,7 +309,7 @@ else
 fi
 
 Msg "check golden images"
-KUBECTL_BINARY=${CMD} INSTALLED_NAMESPACE=${HCO_NAMESPACE} ./hack/check_defaults.sh
+KUBECTL_BINARY=${CMD} INSTALLED_NAMESPACE=${HCO_NAMESPACE} ./hack/check_golden_images.sh
 
 Msg "check virtio-win image is in configmap"
 VIRTIOWIN_IMAGE_CSV=$(${CMD} get ${CSV} -n ${HCO_NAMESPACE} \

pkg/components/components.go

@@ -495,7 +495,7 @@ func GetClusterPermissions() []rbacv1.PolicyRule {
 		{
 			APIGroups: stringListToSlice("image.openshift.io"),
 			Resources: stringListToSlice("imagestreams"),
-			Verbs:     stringListToSlice("get", "list", "watch", "update", "create"),
+			Verbs:     stringListToSlice("get", "list", "watch", "update", "create", "delete"),
 		},
 	}
 }

pkg/controller/hyperconverged/hyperconverged_controller.go

@@ -1119,7 +1119,7 @@ func removeOldQuickStartGuides(req *common.HcoRequest, cl client.Client, require
 		for name, existQs := range existingQSNames {
 			if !hcoutil.ContainsString(requiredQSList, name) {
 				req.Logger.Info("deleting ConsoleQuickStart", "name", name)
-				if err = hcoutil.EnsureDeleted(req.Ctx, cl, &existQs, req.Instance.Name, req.Logger, false, false); err != nil {
+				if _, err = hcoutil.EnsureDeleted(req.Ctx, cl, &existQs, req.Instance.Name, req.Logger, false, false); err != nil {
 					req.Logger.Error(err, "failed to delete ConsoleQuickStart", "name", name)
 				}
 			}

pkg/controller/operands/ensure_result.go

@@ -12,6 +12,7 @@ type EnsureResult struct {
 	Overwritten bool
 	Created     bool
 	UpgradeDone bool
+	Deleted     bool
 	Err         error
 	Type        string
 	Name        string
@@ -52,3 +53,8 @@ func (r *EnsureResult) SetName(name string) *EnsureResult {
 	r.Name = name
 	return r
 }
+
+func (r *EnsureResult) SetDeleted() *EnsureResult {
+	r.Deleted = true
+	return r
+}

pkg/controller/operands/imageStream.go

@@ -7,6 +7,9 @@ import (
 	"reflect"
 	"strings"
 
+	objectreferencesv1 "github.com/openshift/custom-resource-status/objectreferences/v1"
+	"k8s.io/client-go/tools/reference"
+
 	log "github.com/go-logr/logr"
 	imagev1 "github.com/openshift/api/image/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -32,16 +35,56 @@ var (
 	}
 )
 
+type imageStreamOperand struct {
+	operand *genericOperand
+}
+
+func (iso imageStreamOperand) ensure(req *common.HcoRequest) *EnsureResult {
+	if req.Instance.Spec.FeatureGates.EnableCommonBootImageImport {
+		// if the FG is set, make sure the imageStream is in place and up-to-date
+		return iso.operand.ensure(req)
+	}
+
+	// if the FG is not set, make sure the imageStream is not exist
+	cr := iso.operand.hooks.getEmptyCr()
+	res := NewEnsureResult(cr)
+	res.SetName(cr.GetName())
+	deleted, err := util.EnsureDeleted(req.Ctx, iso.operand.Client, cr, req.Instance.Name, req.Logger, false, false)
+	if err != nil {
+		return res.Error(err)
+	}
+
+	if deleted {
+		res.SetDeleted()
+		objectRef, err := reference.GetReference(iso.operand.Scheme, cr)
+		if err != nil {
+			return res.Error(err)
+		}
+
+		if err = objectreferencesv1.RemoveObjectReference(&req.Instance.Status.RelatedObjects, *objectRef); err != nil {
+			return res.Error(err)
+		}
+	}
+
+	return res.SetUpgradeDone(req.ComponentUpgradeInProgress)
+}
+
+func (iso imageStreamOperand) reset() {
+	iso.operand.reset()
+}
+
 func newImageStreamHandler(Client client.Client, Scheme *runtime.Scheme, required *imagev1.ImageStream) Operand {
-	h := &genericOperand{
-		Client: Client,
-		Scheme: Scheme,
-		crType: "ImageStream",
-		// Previous versions used to have HCO-operator (scope namespace)
-		// as the owner of NetworkAddons (scope cluster).
-		// It's not legal, so remove that.
-		removeExistingOwner: false,
-		hooks:               &isHooks{required: required},
+	h := &imageStreamOperand{
+		operand: &genericOperand{
+			Client: Client,
+			Scheme: Scheme,
+			crType: "ImageStream",
+			// Previous versions used to have HCO-operator (scope namespace)
+			// as the owner of NetworkAddons (scope cluster).
+			// It's not legal, so remove that.
+			removeExistingOwner: false,
+			hooks:               &isHooks{required: required},
+		},
 	}
 
 	return h