Migrate ServerStatusRequest controller and resource to kubebuilder (#…

…2838)

* Convert ServerStatusRequest controller to controller-runtime

Signed-off-by: Carlisia <carlisia@vmware.com>

* Add select stm

Signed-off-by: Carlisia <carlisia@vmware.com>

* Fixed status patch bug

Signed-off-by: Carlisia <carlisia@vmware.com>

* Add mgr start

Signed-off-by: Carlisia <carlisia@vmware.com>

* Trying to sync

Signed-off-by: Carlisia <carlisia@vmware.com>

* Clean async now

Signed-off-by: Carlisia <carlisia@vmware.com>

* Clean up + move context out

Signed-off-by: Carlisia <carlisia@vmware.com>

* Bug: not closing the channel

Signed-off-by: Carlisia <carlisia@vmware.com>

* Clean up some tests

Signed-off-by: Carlisia <carlisia@vmware.com>

* Much better way to fetch an update using a backoff loop

Signed-off-by: Carlisia <carlisia@vmware.com>

* Even better way to retry: use apimachinery lib

Signed-off-by: Carlisia <carlisia@vmware.com>

* Refactor controller + add test

Signed-off-by: Carlisia <carlisia@vmware.com>

* partially fix unit tests

Signed-off-by: Ashish Amarnath <ashisham@vmware.com>

* Fix and add tests

Signed-off-by: Carlisia <carlisia@vmware.com>

* Add changelog

Signed-off-by: Carlisia <carlisia@vmware.com>

* Add ability to disable the controller + cleanups

Signed-off-by: Carlisia <carlisia@vmware.com>

* Fix bug w/ disabling controllers + fix test + clean up

Signed-off-by: Carlisia <carlisia@vmware.com>

* Move role.yaml to the correct folder

Signed-off-by: Carlisia <carlisia@vmware.com>

* Add sample serverstatusrequest.yaml

Signed-off-by: Carlisia <carlisia@vmware.com>

* Add requeue + better formatting

Signed-off-by: Carlisia <carlisia@vmware.com>

* Increase # of max concurrent reconciles

Signed-off-by: Carlisia <carlisia@vmware.com>

Co-authored-by: Ashish Amarnath <ashisham@vmware.com>

changelogs/unreleased/2838-carlisia

@@ -0,0 +1 @@
+Convert ServerStatusRequest controller to kubebuilder

config/crd/bases/velero.io_serverstatusrequests.yaml

@@ -16,6 +16,8 @@ spec:
     singular: serverstatusrequest
   preserveUnknownFields: false
   scope: Namespaced
+  subresources:
+    status: {}
   validation:
     openAPIV3Schema:
       description: ServerStatusRequest is a request to access current status information

config/rbac/role.yaml

@@ -26,3 +26,23 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - velero.io
+  resources:
+  - serverstatusrequests
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - velero.io
+  resources:
+  - serverstatusrequests/status
+  verbs:
+  - get
+  - patch
+  - update

config/samples/velero_v1_serverstatusrequest.yaml

@@ -0,0 +1,25 @@
+---
+apiVersion: velero.io/v1
+kind: ServerStatusRequest
+metadata:
+  creationTimestamp: "2020-08-21T15:34:34Z"
+  generateName: velero-cli-
+  generation: 1
+  name: velero-cli-6wkzd
+  namespace: velero
+  resourceVersion: "544749"
+  selfLink: /apis/velero.io/v1/namespaces/velero/serverstatusrequests/velero-cli-6wkzd
+  uid: 335ea64e-1904-40ec-8106-1f2b22e9540e
+spec: {}
+status:
+  phase: Processed
+  plugins:
+  - kind: ObjectStore
+    name: velero.io/aws
+  - kind: VolumeSnapshotter
+    name: velero.io/aws
+  - kind: BackupItemAction
+    name: velero.io/crd-remap-version
+  - kind: BackupItemAction
+    name: velero.io/pod
+  processedTimestamp: "2020-08-21T15:34:34Z"

go.mod

@@ -30,7 +30,9 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.4.0
 	golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7
-	google.golang.org/grpc v1.28.0
+	google.golang.org/genproto v0.0.0-20200731012542-8145dea6a485 // indirect
+	google.golang.org/grpc v1.31.0
+	google.golang.org/protobuf v1.25.0 // indirect
 	k8s.io/api v0.18.4
 	k8s.io/apiextensions-apiserver v0.18.4
 	k8s.io/apimachinery v0.18.4

internal/velero/serverstatusrequest.go

@@ -0,0 +1,73 @@
+/*
+Copyright 2020 the Velero contributors.
+
+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 velero
+
+import (
+	"context"
+
+	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/clock"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/buildinfo"
+	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+)
+
+// ServerStatus holds information for retrieving installed
+// plugins and for updating the ServerStatusRequest timestamp.
+type ServerStatus struct {
+	PluginRegistry PluginLister
+	Clock          clock.Clock
+}
+
+// PatchStatusProcessed patches status fields, including loading the plugin info, and updates
+// the ServerStatusRequest.Status.Phase to ServerStatusRequestPhaseProcessed.
+func (s *ServerStatus) PatchStatusProcessed(kbClient client.Client, req *velerov1api.ServerStatusRequest, ctx context.Context) error {
+	statusPatch := client.MergeFrom(req.DeepCopyObject())
+	req.Status.ServerVersion = buildinfo.Version
+	req.Status.Phase = velerov1api.ServerStatusRequestPhaseProcessed
+	req.Status.ProcessedTimestamp = &metav1.Time{Time: s.Clock.Now()}
+	req.Status.Plugins = getInstalledPluginInfo(s.PluginRegistry)
+
+	if err := kbClient.Status().Patch(ctx, req, statusPatch); err != nil {
+		return errors.WithStack(err)
+	}
+
+	return nil
+}
+
+type PluginLister interface {
+	// List returns all PluginIdentifiers for kind.
+	List(kind framework.PluginKind) []framework.PluginIdentifier
+}
+
+func getInstalledPluginInfo(pluginLister PluginLister) []velerov1api.PluginInfo {
+	var plugins []velerov1api.PluginInfo
+	for _, v := range framework.AllPluginKinds() {
+		list := pluginLister.List(v)
+		for _, plugin := range list {
+			pluginInfo := velerov1api.PluginInfo{
+				Name: plugin.Name,
+				Kind: plugin.Kind.String(),
+			}
+			plugins = append(plugins, pluginInfo)
+		}
+	}
+	return plugins
+}

pkg/serverstatusrequest/process_test.go → internal/velero/serverstatusrequest_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2018 the Velero contributors.
+Copyright 2020 the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -14,33 +14,34 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package serverstatusrequest
+package velero
 
 import (
 	"context"
 	"sort"
 	"testing"
 	"time"
 
-	"github.com/sirupsen/logrus"
+	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/clock"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/builder"
 	"github.com/vmware-tanzu/velero/pkg/buildinfo"
-	"github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/fake"
+	"github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
 )
 
-func statusRequestBuilder() *builder.ServerStatusRequestBuilder {
-	return builder.ForServerStatusRequest(velerov1api.DefaultNamespace, "sr-1")
+func statusRequestBuilder(resourceVersion string) *builder.ServerStatusRequestBuilder {
+	return builder.ForServerStatusRequest(velerov1api.DefaultNamespace, "sr-1", resourceVersion)
 }
 
-func TestProcess(t *testing.T) {
+func TestPatchStatusProcessed(t *testing.T) {
 	// now will be used to set the fake clock's time; capture
 	// it here so it can be referenced in the test case defs.
 	now, err := time.Parse(time.RFC1123, time.RFC1123)
@@ -54,11 +55,10 @@ func TestProcess(t *testing.T) {
 		req             *velerov1api.ServerStatusRequest
 		reqPluginLister *fakePluginLister
 		expected        *velerov1api.ServerStatusRequest
-		expectedErrMsg  string
 	}{
 		{
 			name: "server status request with empty phase gets processed",
-			req:  statusRequestBuilder().Result(),
+			req:  statusRequestBuilder("0").Result(),
 			reqPluginLister: &fakePluginLister{
 				plugins: []framework.PluginIdentifier{
 					{
@@ -67,7 +67,7 @@ func TestProcess(t *testing.T) {
 					},
 				},
 			},
-			expected: statusRequestBuilder().
+			expected: statusRequestBuilder("1").
 				ServerVersion(buildinfo.Version).
 				Phase(velerov1api.ServerStatusRequestPhaseProcessed).
 				ProcessedTimestamp(now).
@@ -81,7 +81,7 @@ func TestProcess(t *testing.T) {
 		},
 		{
 			name: "server status request with phase=New gets processed",
-			req: statusRequestBuilder().
+			req: statusRequestBuilder("0").
 				Phase(velerov1api.ServerStatusRequestPhaseNew).
 				Result(),
 			reqPluginLister: &fakePluginLister{
@@ -96,7 +96,7 @@ func TestProcess(t *testing.T) {
 					},
 				},
 			},
-			expected: statusRequestBuilder().
+			expected: statusRequestBuilder("1").
 				ServerVersion(buildinfo.Version).
 				Phase(velerov1api.ServerStatusRequestPhaseProcessed).
 				ProcessedTimestamp(now).
@@ -113,65 +113,65 @@ func TestProcess(t *testing.T) {
 				Result(),
 		},
 		{
-			name: "server status request with phase=Processed gets deleted if expired",
-			req: statusRequestBuilder().
+			name: "server status request with phase=Processed gets processed",
+			req: statusRequestBuilder("0").
 				Phase(velerov1api.ServerStatusRequestPhaseProcessed).
-				ProcessedTimestamp(now.Add(-61 * time.Second)).
 				Result(),
 			reqPluginLister: &fakePluginLister{
 				plugins: []framework.PluginIdentifier{
+					{
+						Name: "velero.io/aws",
+						Kind: "ObjectStore",
+					},
 					{
 						Name: "custom.io/myown",
 						Kind: "VolumeSnapshotter",
 					},
 				},
 			},
-			expected: nil,
-		},
-		{
-			name: "server status request with phase=Processed does not get deleted if not expired",
-			req: statusRequestBuilder().
-				Phase(velerov1api.ServerStatusRequestPhaseProcessed).
-				ProcessedTimestamp(now.Add(-59 * time.Second)).
-				Result(),
-			expected: statusRequestBuilder().
+			expected: statusRequestBuilder("1").
+				ServerVersion(buildinfo.Version).
 				Phase(velerov1api.ServerStatusRequestPhaseProcessed).
-				ProcessedTimestamp(now.Add(-59 * time.Second)).
-				Result(),
-		},
-		{
-			name: "server status request with invalid phase returns an error",
-			req: statusRequestBuilder().
-				Phase(velerov1api.ServerStatusRequestPhase("an-invalid-phase")).
-				Result(),
-			expected: statusRequestBuilder().
-				Phase(velerov1api.ServerStatusRequestPhase("an-invalid-phase")).
+				ProcessedTimestamp(now).
+				Plugins([]velerov1api.PluginInfo{
+					{
+						Name: "velero.io/aws",
+						Kind: "ObjectStore",
+					},
+					{
+						Name: "custom.io/myown",
+						Kind: "VolumeSnapshotter",
+					},
+				}).
 				Result(),
-			expectedErrMsg: "unexpected ServerStatusRequest phase \"an-invalid-phase\"",
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			client := fake.NewSimpleClientset(tc.req)
+			g := NewWithT(t)
 
-			err := Process(tc.req, client.VeleroV1(), tc.reqPluginLister, clock.NewFakeClock(now), logrus.StandardLogger())
-			if tc.expectedErrMsg == "" {
-				assert.Nil(t, err)
-			} else {
-				assert.EqualError(t, err, tc.expectedErrMsg)
+			serverStatusInfo := ServerStatus{
+				PluginRegistry: tc.reqPluginLister,
+				Clock:          clock.NewFakeClock(now),
 			}
 
-			res, err := client.VeleroV1().ServerStatusRequests(tc.req.Namespace).Get(context.TODO(), tc.req.Name, metav1.GetOptions{})
+			kbClient := fake.NewFakeClientWithScheme(scheme.Scheme, tc.req)
+			err := serverStatusInfo.PatchStatusProcessed(kbClient, tc.req, context.Background())
+			assert.Nil(t, err)
+
+			key := client.ObjectKey{Name: tc.req.Name, Namespace: tc.req.Namespace}
+			instance := &velerov1api.ServerStatusRequest{}
+			err = kbClient.Get(context.Background(), key, instance)
+
 			if tc.expected == nil {
-				assert.Nil(t, res)
-				assert.True(t, apierrors.IsNotFound(err))
+				g.Expect(apierrors.IsNotFound(err)).To(BeTrue())
 			} else {
 				sortPluginsByKindAndName(tc.expected.Status.Plugins)
-				sortPluginsByKindAndName(res.Status.Plugins)
-				assert.Equal(t, tc.expected.Status.Plugins, res.Status.Plugins)
-				assert.Equal(t, tc.expected, res)
-				assert.Nil(t, err)
+				sortPluginsByKindAndName(instance.Status.Plugins)
+				g.Expect(instance.Status.Plugins).To(BeEquivalentTo((tc.expected.Status.Plugins)))
+				g.Expect(instance).To(BeEquivalentTo((tc.expected)))
+				g.Expect(err).To(BeNil())
 			}
 		})
 	}

pkg/apis/velero/v1/server_status_request.go → pkg/apis/velero/v1/server_status_request_types.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2018 the Velero contributors.
+Copyright 2020 the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -20,8 +20,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// TODO(2.0) After converting all resources to use the runttime-controller client,
+// the genclient and k8s:deepcopy markers will no longer be needed and should be removed.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:object:root=true
+// +kubebuilder:object:generate=true
+// +kubebuilder:storageversion
+// +kubebuilder:subresource:status
 
 // ServerStatusRequest is a request to access current status information about
 // the Velero server.
@@ -81,7 +87,12 @@ type ServerStatusRequestStatus struct {
 	Plugins []PluginInfo `json:"plugins,omitempty"`
 }
 
+// TODO(2.0) After converting all resources to use the runttime-controller client,
+// the k8s:deepcopy marker will no longer be needed and should be removed.
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:object:root=true
+// +kubebuilder:rbac:groups=velero.io,resources=serverstatusrequests,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=velero.io,resources=serverstatusrequests/status,verbs=get;update;patch
 
 // ServerStatusRequestList is a list of ServerStatusRequests.
 type ServerStatusRequestList struct {

pkg/builder/server_status_request_builder.go

@@ -30,16 +30,17 @@ type ServerStatusRequestBuilder struct {
 }
 
 // ForServerStatusRequest is the constructor for for a ServerStatusRequestBuilder.
-func ForServerStatusRequest(ns, name string) *ServerStatusRequestBuilder {
+func ForServerStatusRequest(ns, name, resourceVersion string) *ServerStatusRequestBuilder {
 	return &ServerStatusRequestBuilder{
 		object: &velerov1api.ServerStatusRequest{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: velerov1api.SchemeGroupVersion.String(),
 				Kind:       "ServerStatusRequest",
 			},
 			ObjectMeta: metav1.ObjectMeta{
-				Namespace: ns,
-				Name:      name,
+				Namespace:       ns,
+				Name:            name,
+				ResourceVersion: resourceVersion,
 			},
 		},
 	}