fixes

pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go

@@ -8,6 +8,7 @@ import (
 
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 type BannerContent string
@@ -31,6 +32,8 @@ const (
 	GuardRailsStatus          = "GuardRailsStatus"
 )
 
+var SingletonKey = types.NamespacedName{Name: SingletonClusterName}
+
 // AllConditionTypes is a operator conditions currently in use, any condition not in this list is not
 // added to the operator.status.conditions list
 func AllConditionTypes() []string {

pkg/operator/const.go

@@ -1,5 +1,7 @@
 package operator
 
+import "k8s.io/apimachinery/pkg/types"
+
 // Copyright (c) Microsoft Corporation.
 // Licensed under the Apache License 2.0.
 
@@ -10,3 +12,17 @@ const (
 	Namespace  = "openshift-azure-operator"
 	SecretName = "cluster"
 )
+
+var SecretKey = types.NamespacedName{
+	Name:      SecretName,
+	Namespace: Namespace,
+}
+
+var ControlPlaneDeployment = types.NamespacedName{
+	Name:      "aro-operator-master",
+	Namespace: Namespace,
+}
+var WorkerDeployment = types.NamespacedName{
+	Name:      "aro-operator-worker",
+	Namespace: Namespace,
+}

pkg/operator/deploy/deploy.go

@@ -27,6 +27,7 @@ import (
 	appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/util/retry"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/Azure/ARO-RP/pkg/api"
@@ -58,8 +59,8 @@ type operator struct {
 	env env.Interface
 	oc  *api.OpenShiftCluster
 
-	arocli        aroclient.Interface
-	extensionscli extensionsclient.Interface
+	pollTimeout time.Duration
+
 	kubernetescli kubernetes.Interface
 	dh            dynamichelper.Interface
 }
@@ -79,8 +80,8 @@ func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli
 		env: env,
 		oc:  oc,
 
-		arocli:        arocli,
-		extensionscli: extensionscli,
+		pollTimeout: time.Minute,
+
 		kubernetescli: kubernetescli,
 		dh:            dh,
 	}, nil
@@ -282,7 +283,11 @@ func (o *operator) CreateOrUpdate(ctx context.Context) error {
 		return err
 	}
 
-	err = dynamichelper.Prepare(resources)
+	return o.createOrUpdateInner(ctx, resources)
+}
+
+func (o *operator) createOrUpdateInner(ctx context.Context, resources []kruntime.Object) error {
+	err := dynamichelper.Prepare(resources)
 	if err != nil {
 		return err
 	}
@@ -293,28 +298,28 @@ func (o *operator) CreateOrUpdate(ctx context.Context) error {
 			return err
 		}
 
-		gvks, _, err := scheme.Scheme.ObjectKinds(resource)
-		if err != nil {
-			return err
-		}
-
-		switch gvks[0].GroupKind().String() {
-		case "CustomResourceDefinition.apiextensions.k8s.io":
+		c, ok := resource.(client.Object)
+		if !ok {
 			acc, err := meta.Accessor(resource)
 			if err != nil {
 				return err
 			}
+			return fmt.Errorf("unable to handle %s: %s %s", resource.GetObjectKind().GroupVersionKind().String(), acc.GetName(), acc.GetNamespace())
+		}
 
-			err = wait.PollImmediate(time.Second, time.Minute, func() (bool, error) {
-				crd, err := o.extensionscli.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, acc.GetName(), metav1.GetOptions{})
+		switch c.GetObjectKind().GroupVersionKind().GroupKind().String() {
+		case "CustomResourceDefinition.apiextensions.k8s.io":
+			err = wait.PollImmediate(time.Second, o.pollTimeout, func() (bool, error) {
+				crd := &extensionsv1.CustomResourceDefinition{}
+				err := o.dh.GetOne(ctx, client.ObjectKeyFromObject(c), crd)
 				if err != nil {
 					return false, err
 				}
 
 				return isCRDEstablished(crd), nil
 			})
 			if err != nil {
-				return err
+				return fmt.Errorf("CRDs did not establish: %v", err)
 			}
 		case "Cluster.aro.openshift.io":
 			// add an owner reference onto our configuration secret.  This is
@@ -333,12 +338,14 @@ func (o *operator) CreateOrUpdate(ctx context.Context) error {
 				// "aro.openshift.io/v1alpha1"
 				return kerrors.IsForbidden(err) || kerrors.IsConflict(err)
 			}, func() error {
-				cluster, err := o.arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
+				cluster := &arov1alpha1.Cluster{}
+				err := o.dh.GetOne(ctx, arov1alpha1.SingletonKey, cluster)
 				if err != nil {
 					return err
 				}
 
-				s, err := o.kubernetescli.CoreV1().Secrets(pkgoperator.Namespace).Get(ctx, pkgoperator.SecretName, metav1.GetOptions{})
+				s := &corev1.Secret{}
+				err = o.dh.GetOne(ctx, pkgoperator.SecretKey, s)
 				if err != nil {
 					return err
 				}
@@ -348,11 +355,11 @@ func (o *operator) CreateOrUpdate(ctx context.Context) error {
 					return err
 				}
 
-				_, err = o.kubernetescli.CoreV1().Secrets(pkgoperator.Namespace).Update(ctx, s, metav1.UpdateOptions{})
+				err = o.dh.Ensure(ctx, s)
 				return err
 			})
 			if err != nil {
-				return err
+				return fmt.Errorf("unable to add owner to configuration secret: %v", err)
 			}
 		}
 	}
@@ -370,28 +377,25 @@ func (o *operator) RenewMDSDCertificate(ctx context.Context) error {
 		return err
 	}
 
-	s, err := o.kubernetescli.CoreV1().Secrets(pkgoperator.Namespace).Get(ctx, pkgoperator.SecretName, metav1.GetOptions{})
+	s := &corev1.Secret{}
+	err = o.dh.GetOne(ctx, pkgoperator.SecretKey, s)
 	if err != nil {
 		return err
 	}
 	s.Data["gcscert.pem"] = gcsCertBytes
 	s.Data["gcskey.pem"] = gcsKeyBytes
 
-	_, err = o.kubernetescli.CoreV1().Secrets(pkgoperator.Namespace).Update(ctx, s, metav1.UpdateOptions{})
-	if err != nil {
-		return err
-	}
-	return nil
+	return o.dh.Ensure(ctx, s)
 }
 
 func (o *operator) IsReady(ctx context.Context) (bool, error) {
-	ok, err := ready.CheckDeploymentIsReady(ctx, o.kubernetescli.AppsV1().Deployments(pkgoperator.Namespace), "aro-operator-master")()
-	o.log.Infof("deployment %q ok status is: %v, err is: %v", "aro-operator-master", ok, err)
+	ok, err := ready.CheckDeploymentIsReady(ctx, o.dh, pkgoperator.ControlPlaneDeployment)()
+	o.log.Infof("deployment %q ok status is: %v, err is: %v", pkgoperator.ControlPlaneDeployment.Name, ok, err)
 	if !ok || err != nil {
 		return ok, err
 	}
-	ok, err = ready.CheckDeploymentIsReady(ctx, o.kubernetescli.AppsV1().Deployments(pkgoperator.Namespace), "aro-operator-worker")()
-	o.log.Infof("deployment %q ok status is: %v, err is: %v", "aro-operator-worker", ok, err)
+	ok, err = ready.CheckDeploymentIsReady(ctx, o.dh, pkgoperator.WorkerDeployment)()
+	o.log.Infof("deployment %q ok status is: %v, err is: %v", pkgoperator.WorkerDeployment.Name, ok, err)
 	if !ok || err != nil {
 		return ok, err
 	}

pkg/operator/deploy/deploy_test.go

@@ -7,17 +7,25 @@ import (
 	"context"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/golang/mock/gomock"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/Azure/ARO-RP/pkg/api"
+	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/util/cmp"
+	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
 	mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env"
+	testdynamichelper "github.com/Azure/ARO-RP/test/util/dynamichelper"
 	utilerror "github.com/Azure/ARO-RP/test/util/error"
+	testlog "github.com/Azure/ARO-RP/test/util/log"
 )
 
 func TestCheckIngressIP(t *testing.T) {
@@ -399,3 +407,65 @@ func TestCheckPodImageVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestDeploy(t *testing.T) {
+	_, log := testlog.New()
+
+	builder := clientfake.NewClientBuilder().WithRuntimeObjects(&arov1alpha1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: arov1alpha1.SingletonKey.Name,
+		},
+		Spec: arov1alpha1.ClusterSpec{},
+	})
+
+	client := testdynamichelper.NewRedirectingClient(builder.Build()).
+		WithCreateHook(func(obj client.Object) error {
+			if obj.GetObjectKind().GroupVersionKind().String() == "apiextensions.k8s.io/v1, Kind=CustomResourceDefinition" {
+				o := obj.(*extensionsv1.CustomResourceDefinition)
+				o.Status.Conditions = append(o.Status.Conditions,
+					extensionsv1.CustomResourceDefinitionCondition{
+						Type:   extensionsv1.Established,
+						Status: extensionsv1.ConditionTrue,
+					},
+					extensionsv1.CustomResourceDefinitionCondition{
+						Type:   extensionsv1.NamesAccepted,
+						Status: extensionsv1.ConditionTrue,
+					})
+			}
+			return nil
+		})
+
+	dh := dynamichelper.NewWithClient(log, client)
+
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	_env := mock_env.NewMockInterface(controller)
+	_env.EXPECT().ACRDomain().AnyTimes().Return("intsvcdomain")
+	_env.EXPECT().AROOperatorImage().AnyTimes().Return("defaultaroimagefromenv")
+	_env.EXPECT().IsLocalDevelopmentMode().AnyTimes().Return(false)
+
+	oc := &api.OpenShiftCluster{
+		Properties: api.OpenShiftClusterProperties{},
+	}
+
+	d := &operator{
+		log: log,
+		env: _env,
+		oc:  oc,
+
+		pollTimeout: time.Second,
+
+		dh: dh,
+	}
+
+	objs, err := d.createObjects()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = d.createOrUpdateInner(context.Background(), objs)
+	if err != nil {
+		t.Fatal(err)
+	}
+}

pkg/util/ready/ready.go

@@ -13,8 +13,11 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
+
+	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
 )
 
 // NodeIsReady returns true if a Node is considered ready
@@ -86,9 +89,10 @@ func DeploymentIsReady(d *appsv1.Deployment) bool {
 
 // CheckDeploymentIsReady returns a function which polls a Deployment and
 // returns its readiness
-func CheckDeploymentIsReady(ctx context.Context, cli appsv1client.DeploymentInterface, name string) func() (bool, error) {
+func CheckDeploymentIsReady(ctx context.Context, dh dynamichelper.Interface, key types.NamespacedName) func() (bool, error) {
 	return func() (bool, error) {
-		d, err := cli.Get(ctx, name, metav1.GetOptions{})
+		d := &appsv1.Deployment{}
+		err := dh.GetOne(ctx, key, d)
 		switch {
 		case kerrors.IsNotFound(err):
 			return false, nil

pkg/util/ready/ready_test.go

@@ -14,8 +14,14 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 	ktesting "k8s.io/client-go/testing"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
+	testdynamichelper "github.com/Azure/ARO-RP/test/util/dynamichelper"
 )
 
 func TestNodeIsReady(t *testing.T) {
@@ -523,15 +529,12 @@ func TestCheckDeploymentIsReady(t *testing.T) {
 			Namespace: "default",
 		},
 	}
-	clientset := fake.NewSimpleClientset()
-	_, err := clientset.AppsV1().Deployments("default").Create(ctx, deployment, metav1.CreateOptions{})
-	if err != nil {
-		t.Fatalf("error creating deployment: %v", err)
-	}
-	_, err = CheckDeploymentIsReady(ctx, clientset.AppsV1().Deployments("default"), deployment.ObjectMeta.Name)()
+	clientFake := clientfake.NewClientBuilder().WithObjects(deployment).Build()
+	dh := dynamichelper.NewWithClient(nil, clientFake)
+	_, err := CheckDeploymentIsReady(ctx, dh, client.ObjectKeyFromObject(deployment))()
 
 	if err != nil {
-		t.Fatalf("check deployement is not ready: %v", err)
+		t.Fatalf("check deployment is not ready: %v", err)
 	}
 }
 
@@ -543,9 +546,12 @@ func TestCheckDeploymentIsReadyNotFound(t *testing.T) {
 			Namespace: "default",
 		},
 	}
-	clientset := fake.NewSimpleClientset()
-	ok, _ := CheckDeploymentIsReady(ctx, clientset.AppsV1().Deployments("default"), deployment.ObjectMeta.Name)()
-
+	clientFake := clientfake.NewClientBuilder().Build()
+	dh := dynamichelper.NewWithClient(nil, clientFake)
+	ok, err := CheckDeploymentIsReady(ctx, dh, client.ObjectKeyFromObject(deployment))()
+	if err != nil {
+		t.Fatal(err)
+	}
 	if ok {
 		t.Fatalf("check deployment is found")
 	}
@@ -554,11 +560,12 @@ func TestCheckDeploymentIsReadyNotFound(t *testing.T) {
 func TestCheckDeploymentIsReadyError(t *testing.T) {
 	ctx := context.Background()
 
-	clientset := fake.NewSimpleClientset()
-	clientset.Fake.PrependReactor("get", "deployments", func(action ktesting.Action) (bool, kruntime.Object, error) {
-		return true, &appsv1.Deployment{}, errors.New("error getting deployment")
-	})
-	_, err := CheckDeploymentIsReady(ctx, clientset.AppsV1().Deployments("default"), "")()
+	clientFake := testdynamichelper.NewRedirectingClient(clientfake.NewClientBuilder().Build()).
+		WithGetHook(func(key types.NamespacedName, obj client.Object) error {
+			return errors.New("error getting deployment")
+		})
+	dh := dynamichelper.NewWithClient(nil, clientFake)
+	_, err := CheckDeploymentIsReady(ctx, dh, types.NamespacedName{Namespace: "default", Name: "something"})()
 
 	if err == nil {
 		t.Fatalf("check deployment error is: %v", err)