diff --git a/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go b/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go index d8008e1a019..73a4fae5d08 100644 --- a/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go +++ b/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 { diff --git a/pkg/operator/const.go b/pkg/operator/const.go index 384516884aa..79bdd155506 100644 --- a/pkg/operator/const.go +++ b/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, +} diff --git a/pkg/operator/deploy/deploy.go b/pkg/operator/deploy/deploy.go index 3e4695a372c..25896b765f6 100644 --- a/pkg/operator/deploy/deploy.go +++ b/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,20 +298,20 @@ 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 } @@ -314,7 +319,7 @@ func (o *operator) CreateOrUpdate(ctx context.Context) error { 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 } diff --git a/pkg/operator/deploy/deploy_test.go b/pkg/operator/deploy/deploy_test.go index 2f2187f7b2b..f28d7c48329 100644 --- a/pkg/operator/deploy/deploy_test.go +++ b/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) + } +} diff --git a/pkg/util/ready/ready.go b/pkg/util/ready/ready.go index cb1540b3b63..c17f9bca32e 100644 --- a/pkg/util/ready/ready.go +++ b/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 diff --git a/pkg/util/ready/ready_test.go b/pkg/util/ready/ready_test.go index ddcf1ca643f..fcb8c8755dc 100644 --- a/pkg/util/ready/ready_test.go +++ b/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)