Move install routine into common code and add tests.

pkg/reconciler/common/install.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+	"fmt"
+
+	mf "github.com/manifestival/manifestival"
+	"knative.dev/operator/pkg/apis/operator/v1alpha1"
+)
+
+var (
+	role        mf.Predicate = mf.Any(mf.ByKind("ClusterRole"), mf.ByKind("Role"))
+	rolebinding mf.Predicate = mf.Any(mf.ByKind("ClusterRoleBinding"), mf.ByKind("RoleBinding"))
+)
+
+// Install applies the manifest resources for the given version and updates the given
+// status accordingly.
+func Install(manifest *mf.Manifest, version string, status v1alpha1.KComponentStatus) error {
+	// The Operator needs a higher level of permissions if it 'bind's non-existent roles.
+	// To avoid this, we strictly order the manifest application as (Cluster)Roles, then
+	// (Cluster)RoleBindings, then the rest of the manifest.
+	if err := manifest.Filter(role).Apply(); err != nil {
+		status.MarkInstallFailed(err.Error())
+		return fmt.Errorf("failed to apply (cluster)roles: %w", err)
+	}
+	if err := manifest.Filter(rolebinding).Apply(); err != nil {
+		status.MarkInstallFailed(err.Error())
+		return fmt.Errorf("failed to apply (cluster)rolebindings: %w", err)
+	}
+	if err := manifest.Filter(mf.None(role, rolebinding)).Apply(); err != nil {
+		status.MarkInstallFailed(err.Error())
+		return fmt.Errorf("failed to apply non rbac manifest: %w", err)
+	}
+	status.MarkInstallSucceeded()
+	status.SetVersion(version)
+	return nil
+}

pkg/reconciler/common/install_test.go

@@ -0,0 +1,121 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	mf "github.com/manifestival/manifestival"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"knative.dev/operator/pkg/apis/operator/v1alpha1"
+)
+
+func TestInstall(t *testing.T) {
+	// Resources in the manifest
+	version := "v0.14-test"
+	deployment := NamespacedResource("apps/v1", "Deployment", "test", "test-deployment")
+	role := NamespacedResource("rbac.authorization.k8s.io/v1", "Role", "test", "test-role")
+	roleBinding := NamespacedResource("rbac.authorization.k8s.io/v1", "RoleBinding", "test", "test-role-binding")
+	clusterRole := ClusterScopedResource("rbac.authorization.k8s.io/v1", "ClusterRole", "test-cluster-role")
+	clusterRoleBinding := ClusterScopedResource("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "test-cluster-role-binding")
+
+	// Deliberately mixing the order in the manifest.
+	in := []unstructured.Unstructured{*deployment, *role, *roleBinding, *clusterRole, *clusterRoleBinding}
+	// Expect things to be applied in order.
+	want := []*unstructured.Unstructured{role, clusterRole, roleBinding, clusterRoleBinding, deployment}
+
+	client := &fakeClient{}
+	manifest, err := mf.ManifestFrom(mf.Slice(in), mf.UseClient(client))
+	if err != nil {
+		t.Fatalf("Failed to generate manifest: %v", err)
+	}
+
+	status := &v1alpha1.KnativeServingStatus{
+		Version: "0.13-test",
+	}
+	if err := Install(&manifest, version, status); err != nil {
+		t.Fatalf("Install() = %v, want no error", err)
+	}
+
+	if !cmp.Equal(client.creates, want) {
+		t.Fatalf("Unexpected creates: %s", cmp.Diff(client.creates, want))
+	}
+
+	condition := status.GetCondition(v1alpha1.InstallSucceeded)
+	if condition == nil || condition.Status != corev1.ConditionTrue {
+		t.Fatalf("InstallSucceeded = %v, want %v", condition, corev1.ConditionTrue)
+	}
+
+	if got, want := status.GetVersion(), version; got != want {
+		t.Fatalf("GetVersion() = %s, want %s", got, want)
+	}
+}
+
+func TestInstallError(t *testing.T) {
+	oldVersion := "v0.13-test"
+	version := "v0.14-test"
+
+	client := &fakeClient{err: errors.New("test")}
+	manifest, err := mf.ManifestFrom(mf.Slice([]unstructured.Unstructured{
+		*NamespacedResource("apps/v1", "Deployment", "test", "test-deployment"),
+	}), mf.UseClient(client))
+	if err != nil {
+		t.Fatalf("Failed to generate manifest: %v", err)
+	}
+
+	status := &v1alpha1.KnativeServingStatus{
+		Version: oldVersion,
+	}
+	if err := Install(&manifest, version, status); err == nil {
+		t.Fatalf("Install() = nil, wanted an error")
+	}
+
+	condition := status.GetCondition(v1alpha1.InstallSucceeded)
+	if condition == nil || condition.Status != corev1.ConditionFalse {
+		t.Fatalf("InstallSucceeded = %v, want %v", condition, corev1.ConditionFalse)
+	}
+
+	if got, want := status.GetVersion(), oldVersion; got != want {
+		t.Fatalf("GetVersion() = %s, want %s", got, want)
+	}
+}
+
+type fakeClient struct {
+	err     error
+	creates []*unstructured.Unstructured
+}
+
+func (f *fakeClient) Get(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
+	return nil, f.err
+}
+
+func (f *fakeClient) Delete(obj *unstructured.Unstructured, options ...mf.DeleteOption) error {
+	return f.err
+}
+
+func (f *fakeClient) Create(obj *unstructured.Unstructured, options ...mf.ApplyOption) error {
+	obj.SetAnnotations(nil) // Deleting the extra annotation. Irrelevant for the test.
+	f.creates = append(f.creates, obj)
+	return f.err
+}
+
+func (f *fakeClient) Update(obj *unstructured.Unstructured, options ...mf.ApplyOption) error {
+	return f.err
+}

pkg/reconciler/knativeeventing/knativeeventing.go

@@ -188,24 +188,7 @@ func (r *Reconciler) ensureFinalizerRemoval(_ context.Context, _ *mf.Manifest, i
 func (r *Reconciler) install(ctx context.Context, manifest *mf.Manifest, ke *eventingv1alpha1.KnativeEventing) error {
 	logger := logging.FromContext(ctx)
 	logger.Debug("Installing manifest")
-	// The Operator needs a higher level of permissions if it 'bind's non-existent roles.
-	// To avoid this, we strictly order the manifest application as (Cluster)Roles, then
-	// (Cluster)RoleBindings, then the rest of the manifest.
-	if err := manifest.Filter(role).Apply(); err != nil {
-		ke.Status.MarkInstallFailed(err.Error())
-		return err
-	}
-	if err := manifest.Filter(rolebinding).Apply(); err != nil {
-		ke.Status.MarkInstallFailed(err.Error())
-		return err
-	}
-	if err := manifest.Filter(mf.None(role, rolebinding)).Apply(); err != nil {
-		ke.Status.MarkInstallFailed(err.Error())
-		return err
-	}
-	ke.Status.MarkInstallSucceeded()
-	ke.Status.Version = version.EventingVersion
-	return nil
+	return common.Install(manifest, version.EventingVersion, &ke.Status)
 }
 
 func (r *Reconciler) checkDeployments(ctx context.Context, manifest *mf.Manifest, ke *eventingv1alpha1.KnativeEventing) error {

pkg/reconciler/knativeserving/knativeserving.go

@@ -162,26 +162,8 @@ func (r *Reconciler) transform(ctx context.Context, instance *servingv1alpha1.Kn
 // Apply the manifest resources
 func (r *Reconciler) install(ctx context.Context, manifest *mf.Manifest, instance *servingv1alpha1.KnativeServing) error {
 	logger := logging.FromContext(ctx)
-
 	logger.Debug("Installing manifest")
-	// The Operator needs a higher level of permissions if it 'bind's non-existent roles.
-	// To avoid this, we strictly order the manifest application as (Cluster)Roles, then
-	// (Cluster)RoleBindings, then the rest of the manifest.
-	if err := manifest.Filter(role).Apply(); err != nil {
-		instance.Status.MarkInstallFailed(err.Error())
-		return err
-	}
-	if err := manifest.Filter(rolebinding).Apply(); err != nil {
-		instance.Status.MarkInstallFailed(err.Error())
-		return err
-	}
-	if err := manifest.Apply(); err != nil {
-		instance.Status.MarkInstallFailed(err.Error())
-		return err
-	}
-	instance.Status.MarkInstallSucceeded()
-	instance.Status.Version = version.ServingVersion
-	return nil
+	return common.Install(manifest, version.ServingVersion, &instance.Status)
 }
 
 // Check for all deployments available