From a897eee4026789ed0535dd57a6bdbb6391ab7cd8 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sun, 15 Mar 2020 18:08:11 +0900 Subject: [PATCH] Fix RBAC role for RunnerDeployment and RunnerReplicaSet --- api/v1alpha1/runner_types.go | 4 +- ...ions.summerwind.dev_runnerdeployments.yaml | 2 +- ...ions.summerwind.dev_runnerreplicasets.yaml | 2 +- config/rbac/role.yaml | 40 +++++++++---------- controllers/runnerdeployment_controller.go | 29 +++++++------- .../runnerdeployment_controller_test.go | 18 +++++---- controllers/runnerreplicaset_controller.go | 9 +++-- .../runnerreplicaset_controller_test.go | 20 ++++++---- 8 files changed, 67 insertions(+), 57 deletions(-) diff --git a/api/v1alpha1/runner_types.go b/api/v1alpha1/runner_types.go index 9c19cab8..23c174a5 100644 --- a/api/v1alpha1/runner_types.go +++ b/api/v1alpha1/runner_types.go @@ -94,7 +94,7 @@ type RunnerList struct { // +kubebuilder:printcolumn:JSONPath=".status.availableReplicas",name=Current,type=number // +kubebuilder:printcolumn:JSONPath=".status.readyReplicas",name=Ready,type=number -// RunnerReplicaSet is the Schema for the runnersets API +// RunnerReplicaSet is the Schema for the runnerreplicasets API type RunnerReplicaSet struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -136,7 +136,7 @@ type RunnerReplicaSetList struct { // +kubebuilder:printcolumn:JSONPath=".status.availableReplicas",name=Current,type=number // +kubebuilder:printcolumn:JSONPath=".status.readyReplicas",name=Ready,type=number -// RunnerReplicaSet is the Schema for the runnersets API +// RunnerDeployment is the Schema for the runnerdeployments API type RunnerDeployment struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml index 0b18fddf..cfd372e5 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml @@ -29,7 +29,7 @@ spec: status: {} validation: openAPIV3Schema: - description: RunnerReplicaSet is the Schema for the runnersets API + description: RunnerDeployment is the Schema for the runnerdeployments API properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation diff --git a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml b/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml index 0014dcab..32b02964 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml @@ -29,7 +29,7 @@ spec: status: {} validation: openAPIV3Schema: - description: RunnerReplicaSet is the Schema for the runnersets API + description: RunnerReplicaSet is the Schema for the runnerreplicasets API properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index eba62d65..49606919 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -26,6 +26,26 @@ rules: - get - patch - update +- apiGroups: + - actions.summerwind.dev + resources: + - runnerreplicasets + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - actions.summerwind.dev + resources: + - runnerreplicasets/status + verbs: + - get + - patch + - update - apiGroups: - actions.summerwind.dev resources: @@ -46,26 +66,6 @@ rules: - get - patch - update -- apiGroups: - - actions.summerwind.dev - resources: - - runnersets - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - actions.summerwind.dev - resources: - - runnersets/status - verbs: - - get - - patch - - update - apiGroups: - "" resources: diff --git a/controllers/runnerdeployment_controller.go b/controllers/runnerdeployment_controller.go index 1b3148f9..19654353 100644 --- a/controllers/runnerdeployment_controller.go +++ b/controllers/runnerdeployment_controller.go @@ -19,15 +19,16 @@ package controllers import ( "context" "fmt" + "hash/fnv" + "sort" + "github.com/davecgh/go-spew/spew" "github.com/go-logr/logr" - "hash/fnv" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sort" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -51,12 +52,12 @@ type RunnerDeploymentReconciler struct { // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerdeployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerdeployments/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnersets,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnersets/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerreplicasets,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerreplicasets/status,verbs=get;update;patch func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { ctx := context.Background() - log := r.Log.WithValues("runnerset", req.NamespacedName) + log := r.Log.WithValues("runnerreplicaset", req.NamespacedName) var rd v1alpha1.RunnerDeployment if err := r.Get(ctx, req.NamespacedName, &rd); err != nil { @@ -92,14 +93,14 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e desiredRS, err := r.newRunnerReplicaSet(rd) if err != nil { - log.Error(err, "Could not create runnerset") + log.Error(err, "Could not create runnerreplicaset") return ctrl.Result{}, err } if newestSet == nil { if err := r.Client.Create(ctx, &desiredRS); err != nil { - log.Error(err, "Failed to create runnerset resource") + log.Error(err, "Failed to create runnerreplicaset resource") return ctrl.Result{}, err } @@ -109,21 +110,21 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e newestTemplateHash, ok := getTemplateHash(newestSet) if !ok { - log.Info("Failed to get template hash of newest runnerset resource. It must be in an invalid state. Please manually delete the runnerset so that it is recreated") + log.Info("Failed to get template hash of newest runnerreplicaset resource. It must be in an invalid state. Please manually delete the runnerreplicaset so that it is recreated") return ctrl.Result{}, nil } desiredTemplateHash, ok := getTemplateHash(&desiredRS) if !ok { - log.Info("Failed to get template hash of desired runnerset resource. It must be in an invalid state. Please manually delete the runnerset so that it is recreated") + log.Info("Failed to get template hash of desired runnerreplicaset resource. It must be in an invalid state. Please manually delete the runnerreplicaset so that it is recreated") return ctrl.Result{}, nil } if newestTemplateHash != desiredTemplateHash { if err := r.Client.Create(ctx, &desiredRS); err != nil { - log.Error(err, "Failed to create runnerset resource") + log.Error(err, "Failed to create runnerreplicaset resource") return ctrl.Result{}, err } @@ -131,12 +132,12 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e return ctrl.Result{}, nil } - // Please add more conditions that we can in-place update the newest runnerset without disruption + // Please add more conditions that we can in-place update the newest runnerreplicaset without disruption if newestSet.Spec.Replicas != desiredRS.Spec.Replicas { newestSet.Spec.Replicas = desiredRS.Spec.Replicas if err := r.Client.Update(ctx, newestSet); err != nil { - log.Error(err, "Failed to update runnerset resource") + log.Error(err, "Failed to update runnerreplicaset resource") return ctrl.Result{}, err } @@ -153,8 +154,8 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e return ctrl.Result{}, err } - r.Recorder.Event(&rd, corev1.EventTypeNormal, "RunnerReplicaSetDeleted", fmt.Sprintf("Deleted runnerset '%s'", rs.Name)) - log.Info("Deleted runnerset", "runnerdeployment", rd.ObjectMeta.Name, "runnerset", rs.Name) + r.Recorder.Event(&rd, corev1.EventTypeNormal, "RunnerReplicaSetDeleted", fmt.Sprintf("Deleted runnerreplicaset '%s'", rs.Name)) + log.Info("Deleted runnerreplicaset", "runnerdeployment", rd.ObjectMeta.Name, "runnerreplicaset", rs.Name) } return ctrl.Result{}, nil diff --git a/controllers/runnerdeployment_controller_test.go b/controllers/runnerdeployment_controller_test.go index a668de12..a0893eca 100644 --- a/controllers/runnerdeployment_controller_test.go +++ b/controllers/runnerdeployment_controller_test.go @@ -2,12 +2,13 @@ package controllers import ( "context" + "time" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" - "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -43,7 +44,7 @@ func SetupDeploymentTest(ctx context.Context) *corev1.Namespace { Client: mgr.GetClient(), Scheme: scheme.Scheme, Log: logf.Log, - Recorder: mgr.GetEventRecorderFor("runnerset-controller"), + Recorder: mgr.GetEventRecorderFor("runnerreplicaset-controller"), } err = controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -72,7 +73,7 @@ var _ = Context("Inside of a new namespace", func() { Describe("when no existing resources exist", func() { - It("should create a new RunnerReplicaSet resource from the specified template, add a another RunnerReplicaSet on template modification, and eventually removes old runnersets", func() { + It("should create a new RunnerReplicaSet resource from the specified template, add a another RunnerReplicaSet on template modification, and eventually removes old runnerreplicasets", func() { name := "example-runnerdeploy" { @@ -120,7 +121,7 @@ var _ = Context("Inside of a new namespace", func() { } if len(runnerSets.Items) == 0 { - logf.Log.Info("No runnersets exist yet") + logf.Log.Info("No runnerreplicasets exist yet") return -1 } @@ -130,9 +131,12 @@ var _ = Context("Inside of a new namespace", func() { } { - // We wrap the update in the Eventually block to avoid the below error that occurs due to concurrent modification - // made by the controller to update .Status.AvailableReplicas and .Status.ReadyReplicas - // Operation cannot be fulfilled on runnersets.actions.summerwind.dev "example-runnerset": the object has been modified; please apply your changes to the latest version and try again + // We wrap the update in the Eventually block to avoid the below error + // that occurs due to concurrent modification made by the controller + // to update .Status.AvailableReplicas and .Status.ReadyReplicas + // Operation cannot be fulfilled on runnerreplicasets.actions.summerwind.dev + // "example-runnerreplciaset": the object has been modified; please + // apply your changes to the latest version and try again. Eventually(func() error { var rd actionsv1alpha1.RunnerDeployment diff --git a/controllers/runnerreplicaset_controller.go b/controllers/runnerreplicaset_controller.go index 4633f60d..fa92172c 100644 --- a/controllers/runnerreplicaset_controller.go +++ b/controllers/runnerreplicaset_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -40,8 +41,8 @@ type RunnerReplicaSetReconciler struct { Scheme *runtime.Scheme } -// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnersets,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnersets/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerreplicasets,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerreplicasets/status,verbs=get;update;patch // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners/status,verbs=get;update;patch @@ -102,7 +103,7 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e } r.Recorder.Event(&rs, corev1.EventTypeNormal, "RunnerDeleted", fmt.Sprintf("Deleted runner '%s'", myRunners[i].Name)) - log.Info("Deleted runner", "runnerset", rs.ObjectMeta.Name) + log.Info("Deleted runner", "runnerreplicaset", rs.ObjectMeta.Name) } } else if desired > available { n := desired - available @@ -157,7 +158,7 @@ func (r *RunnerReplicaSetReconciler) newRunner(rs v1alpha1.RunnerReplicaSet) (v1 } func (r *RunnerReplicaSetReconciler) SetupWithManager(mgr ctrl.Manager) error { - r.Recorder = mgr.GetEventRecorderFor("runnerset-controller") + r.Recorder = mgr.GetEventRecorderFor("runnerreplicaset-controller") return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.RunnerReplicaSet{}). diff --git a/controllers/runnerreplicaset_controller_test.go b/controllers/runnerreplicaset_controller_test.go index 5e6a3a17..b96ba57c 100644 --- a/controllers/runnerreplicaset_controller_test.go +++ b/controllers/runnerreplicaset_controller_test.go @@ -2,13 +2,14 @@ package controllers import ( "context" + "math/rand" + "time" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" - "math/rand" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" - "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -44,7 +45,7 @@ func SetupTest(ctx context.Context) *corev1.Namespace { Client: mgr.GetClient(), Scheme: scheme.Scheme, Log: logf.Log, - Recorder: mgr.GetEventRecorderFor("runnerset-controller"), + Recorder: mgr.GetEventRecorderFor("runnerreplicaset-controller"), } err = controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -88,7 +89,7 @@ var _ = Context("Inside of a new namespace", func() { Describe("when no existing resources exist", func() { It("should create a new Runner resource from the specified template, add a another Runner on replicas increased, and removes all the replicas when set to 0", func() { - name := "example-runnerset" + name := "example-runnerreplicaset" { rs := &actionsv1alpha1.RunnerReplicaSet{ @@ -131,7 +132,7 @@ var _ = Context("Inside of a new namespace", func() { { // We wrap the update in the Eventually block to avoid the below error that occurs due to concurrent modification // made by the controller to update .Status.AvailableReplicas and .Status.ReadyReplicas - // Operation cannot be fulfilled on runnersets.actions.summerwind.dev "example-runnerset": the object has been modified; please apply your changes to the latest version and try again + // Operation cannot be fulfilled on runnerreplicasets.actions.summerwind.dev "example-runnerreplicaset": the object has been modified; please apply your changes to the latest version and try again Eventually(func() error { var rs actionsv1alpha1.RunnerReplicaSet @@ -160,9 +161,12 @@ var _ = Context("Inside of a new namespace", func() { } { - // We wrap the update in the Eventually block to avoid the below error that occurs due to concurrent modification - // made by the controller to update .Status.AvailableReplicas and .Status.ReadyReplicas - // Operation cannot be fulfilled on runnersets.actions.summerwind.dev "example-runnerset": the object has been modified; please apply your changes to the latest version and try again + // We wrap the update in the Eventually block to avoid the below error + // that occurs due to concurrent modification made by the controller + // to update .Status.AvailableReplicas and .Status.ReadyReplicas + // Operation cannot be fulfilled on runnerreplicasets.actions.summerwind.dev + // "example-runnerreplicaset": the object has been modified; please apply + // your changes to the latest version and try again. Eventually(func() error { var rs actionsv1alpha1.RunnerReplicaSet