From b24292170f056924de9e5b7b186b1e0683631ab0 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Mon, 7 Sep 2020 18:51:51 +0100 Subject: [PATCH 1/6] Error string shouldn't be capitalized --- controllers/rabbitmqcluster_controller.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/rabbitmqcluster_controller.go b/controllers/rabbitmqcluster_controller.go index e62aeef0e..f2afe5310 100644 --- a/controllers/rabbitmqcluster_controller.go +++ b/controllers/rabbitmqcluster_controller.go @@ -459,13 +459,13 @@ func (r *RabbitmqClusterReconciler) prepareForDeletion(ctx context.Context, rabb } // Add label on all Pods to be picked up in pre-stop hook via Downward API if err := r.addRabbitmqDeletionLabel(ctx, rabbitmqCluster); err != nil { - return fmt.Errorf("Failed to add deletion markers to RabbitmqCluster Pods: %s", err.Error()) + return fmt.Errorf("failed to add deletion markers to RabbitmqCluster Pods: %s", err.Error()) } // Delete StatefulSet immediately after changing pod labels to minimize risk of them respawning. // There is a window where the StatefulSet could respawn Pods without the deletion label in this order. // But we can't delete it before because the DownwardAPI doesn't update once a Pod enters Terminating. if err := r.Client.Delete(ctx, sts); client.IgnoreNotFound(err) != nil { - return fmt.Errorf("Cannot delete StatefulSet: %s", err.Error()) + return fmt.Errorf("cannot delete StatefulSet: %s", err.Error()) } return nil @@ -508,7 +508,7 @@ func (r *RabbitmqClusterReconciler) addRabbitmqDeletionLabel(ctx context.Context pod := &pods.Items[i] pod.Labels[resource.DeletionMarker] = "true" if err := r.Client.Update(ctx, pod); client.IgnoreNotFound(err) != nil { - return fmt.Errorf("Cannot Update Pod %s in Namespace %s: %s", pod.Name, pod.Namespace, err.Error()) + return fmt.Errorf("cannot Update Pod %s in Namespace %s: %s", pod.Name, pod.Namespace, err.Error()) } } From b44f0aae9e3ad07f03a436f174414cf43c5130a8 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Mon, 7 Sep 2020 18:58:08 +0100 Subject: [PATCH 2/6] Remove redundant package alias --- api/v1beta1/rabbitmqcluster_types.go | 2 +- api/v1beta1/rabbitmqcluster_types_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v1beta1/rabbitmqcluster_types.go b/api/v1beta1/rabbitmqcluster_types.go index 5d779150e..23dd17a26 100644 --- a/api/v1beta1/rabbitmqcluster_types.go +++ b/api/v1beta1/rabbitmqcluster_types.go @@ -18,7 +18,7 @@ import ( corev1 "k8s.io/api/core/v1" k8sresource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" ) const ( diff --git a/api/v1beta1/rabbitmqcluster_types_test.go b/api/v1beta1/rabbitmqcluster_types_test.go index f95d086cc..28f220f13 100644 --- a/api/v1beta1/rabbitmqcluster_types_test.go +++ b/api/v1beta1/rabbitmqcluster_types_test.go @@ -16,7 +16,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" k8sresource "k8s.io/apimachinery/pkg/api/resource" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" "golang.org/x/net/context" apierrors "k8s.io/apimachinery/pkg/api/errors" From ae199bb5b82f6a4edcba945e8ac954b3eac2027f Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Mon, 7 Sep 2020 18:58:49 +0100 Subject: [PATCH 3/6] Rename receivers for consistency in package api/ --- api/v1beta1/rabbitmqcluster_types.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/api/v1beta1/rabbitmqcluster_types.go b/api/v1beta1/rabbitmqcluster_types.go index 23dd17a26..5c8e1556c 100644 --- a/api/v1beta1/rabbitmqcluster_types.go +++ b/api/v1beta1/rabbitmqcluster_types.go @@ -325,13 +325,13 @@ func (cluster *RabbitmqCluster) AdditionalPluginEnabled(plugin Plugin) bool { return false } -func (rmqStatus *RabbitmqClusterStatus) SetConditions(resources []runtime.Object) { +func (clusterStatus *RabbitmqClusterStatus) SetConditions(resources []runtime.Object) { var oldAllPodsReadyCondition *status.RabbitmqClusterCondition var oldClusterAvailableCondition *status.RabbitmqClusterCondition var oldNoWarningsCondition *status.RabbitmqClusterCondition var oldReconcileCondition *status.RabbitmqClusterCondition - for _, condition := range rmqStatus.Conditions { + for _, condition := range clusterStatus.Conditions { switch condition.Type { case status.AllReplicasReady: oldAllPodsReadyCondition = condition.DeepCopy() @@ -355,7 +355,7 @@ func (rmqStatus *RabbitmqClusterStatus) SetConditions(resources []runtime.Object reconciledCondition = status.ReconcileSuccessCondition(corev1.ConditionUnknown, "Initialising", "") } - rmqStatus.Conditions = []status.RabbitmqClusterCondition{ + clusterStatus.Conditions = []status.RabbitmqClusterCondition{ allReplicasReadyCond, clusterAvailableCond, noWarningsCond, @@ -363,12 +363,12 @@ func (rmqStatus *RabbitmqClusterStatus) SetConditions(resources []runtime.Object } } -func (rmqStatus *RabbitmqClusterStatus) SetCondition(condType status.RabbitmqClusterConditionType, +func (clusterStatus *RabbitmqClusterStatus) SetCondition(condType status.RabbitmqClusterConditionType, condStatus corev1.ConditionStatus, reason string, messages ...string) { - for i := range rmqStatus.Conditions { - if rmqStatus.Conditions[i].Type == condType { - rmqStatus.Conditions[i].UpdateState(condStatus) - rmqStatus.Conditions[i].UpdateReason(reason, messages...) + for i := range clusterStatus.Conditions { + if clusterStatus.Conditions[i].Type == condType { + clusterStatus.Conditions[i].UpdateState(condStatus) + clusterStatus.Conditions[i].UpdateReason(reason, messages...) break } } @@ -383,8 +383,8 @@ type RabbitmqClusterList struct { Items []RabbitmqCluster `json:"items"` } -func (r RabbitmqCluster) ChildResourceName(name string) string { - return strings.Join([]string{r.Name, "rabbitmq", name}, "-") +func (cluster RabbitmqCluster) ChildResourceName(name string) string { + return strings.Join([]string{cluster.Name, "rabbitmq", name}, "-") } func init() { From 124636681153ac15ccbf169e2572cdf20eac429f Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Mon, 7 Sep 2020 18:59:54 +0100 Subject: [PATCH 4/6] Remove redundant type conversions and annotations --- api/v1beta1/rabbitmqcluster_types_test.go | 12 ++++++------ internal/resource/client_service.go | 2 +- internal/resource/client_service_test.go | 8 ++++---- internal/resource/statefulset_test.go | 6 +++--- internal/status/status_test.go | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/api/v1beta1/rabbitmqcluster_types_test.go b/api/v1beta1/rabbitmqcluster_types_test.go index 28f220f13..bae70d90c 100644 --- a/api/v1beta1/rabbitmqcluster_types_test.go +++ b/api/v1beta1/rabbitmqcluster_types_test.go @@ -163,7 +163,7 @@ var _ = Describe("RabbitmqCluster", func() { Image: "rabbitmq-image-from-cr", ImagePullSecret: "my-super-secret", Service: RabbitmqClusterServiceSpec{ - Type: corev1.ServiceType("this-is-a-service"), + Type: "this-is-a-service", Annotations: map[string]string{ "myannotation": "is-set", }, @@ -186,7 +186,7 @@ var _ = Describe("RabbitmqCluster", func() { NodeAffinity: &corev1.NodeAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ NodeSelectorTerms: []corev1.NodeSelectorTerm{ - corev1.NodeSelectorTerm{ + { MatchExpressions: []corev1.NodeSelectorRequirement{ { Key: "somekey", @@ -201,7 +201,7 @@ var _ = Describe("RabbitmqCluster", func() { }, }, Tolerations: []corev1.Toleration{ - corev1.Toleration{ + { Key: "mykey", Operator: "NotEqual", Value: "myvalue", @@ -325,15 +325,15 @@ var _ = Describe("RabbitmqCluster", func() { It("updates an arbitrary condition", func() { someCondition := status.RabbitmqClusterCondition{} - someCondition.Type = status.RabbitmqClusterConditionType("a-type") + someCondition.Type = "a-type" someCondition.Reason = "whynot" - someCondition.Status = corev1.ConditionStatus("perhaps") + someCondition.Status = "perhaps" someCondition.LastTransitionTime = metav1.Unix(10, 0) rmqStatus := RabbitmqClusterStatus{ Conditions: []status.RabbitmqClusterCondition{someCondition}, } - rmqStatus.SetCondition(status.RabbitmqClusterConditionType("a-type"), + rmqStatus.SetCondition("a-type", corev1.ConditionTrue, "some-reason", "my-message") updatedCondition := rmqStatus.Conditions[0] diff --git a/internal/resource/client_service.go b/internal/resource/client_service.go index 9c3eb65cc..17b6cc161 100644 --- a/internal/resource/client_service.go +++ b/internal/resource/client_service.go @@ -52,7 +52,7 @@ func (builder *ClientServiceBuilder) Update(object runtime.Object) error { service := object.(*corev1.Service) builder.setAnnotations(service) service.Labels = metadata.GetLabels(builder.Instance.Name, builder.Instance.Labels) - service.Spec.Type = corev1.ServiceType(builder.Instance.Spec.Service.Type) + service.Spec.Type = builder.Instance.Spec.Service.Type service.Spec.Selector = metadata.LabelSelector(builder.Instance.Name) service.Spec.Ports = builder.updatePorts(service.Spec.Ports) diff --git a/internal/resource/client_service_test.go b/internal/resource/client_service_test.go index d67934217..f2fff68b9 100644 --- a/internal/resource/client_service_test.go +++ b/internal/resource/client_service_test.go @@ -334,13 +334,13 @@ var _ = Context("ClientServices", func() { It("preserves the same node ports after updating from LoadBalancer to NodePort", func() { svc.Spec.Type = corev1.ServiceTypeLoadBalancer svc.Spec.Ports = []corev1.ServicePort{ - corev1.ServicePort{ + { Protocol: corev1.ProtocolTCP, Port: 5672, Name: "amqp", NodePort: 12345, }, - corev1.ServicePort{ + { Protocol: corev1.ProtocolTCP, Port: 15672, Name: "management", @@ -372,7 +372,7 @@ var _ = Context("ClientServices", func() { It("unsets nodePort after updating from NodePort to ClusterIP", func() { svc.Spec.Type = corev1.ServiceTypeNodePort svc.Spec.Ports = []corev1.ServicePort{ - corev1.ServicePort{ + { Protocol: corev1.ProtocolTCP, Port: 5672, Name: "amqp", @@ -399,7 +399,7 @@ var _ = Context("ClientServices", func() { It("unsets the service type and node ports when service type is deleted from CR spec", func() { svc.Spec.Type = corev1.ServiceTypeNodePort svc.Spec.Ports = []corev1.ServicePort{ - corev1.ServicePort{ + { Protocol: corev1.ProtocolTCP, Port: 5672, Name: "amqp", diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index 401ba7639..c3603696e 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -1409,7 +1409,7 @@ func generateRabbitmqCluster() rabbitmqv1beta1.RabbitmqCluster { Image: "rabbitmq-image-from-cr", ImagePullSecret: "my-super-secret", Service: rabbitmqv1beta1.RabbitmqClusterServiceSpec{ - Type: corev1.ServiceType("this-is-a-service"), + Type: "this-is-a-service", Annotations: map[string]string{}, }, Persistence: rabbitmqv1beta1.RabbitmqClusterPersistenceSpec{ @@ -1430,7 +1430,7 @@ func generateRabbitmqCluster() rabbitmqv1beta1.RabbitmqCluster { NodeAffinity: &corev1.NodeAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ NodeSelectorTerms: []corev1.NodeSelectorTerm{ - corev1.NodeSelectorTerm{ + { MatchExpressions: []corev1.NodeSelectorRequirement{ { Key: "somekey", @@ -1445,7 +1445,7 @@ func generateRabbitmqCluster() rabbitmqv1beta1.RabbitmqCluster { }, }, Tolerations: []corev1.Toleration{ - corev1.Toleration{ + { Key: "mykey", Operator: "NotEqual", Value: "myvalue", diff --git a/internal/status/status_test.go b/internal/status/status_test.go index 403438834..e8537c9cf 100644 --- a/internal/status/status_test.go +++ b/internal/status/status_test.go @@ -20,8 +20,8 @@ var _ = Describe("Status", func() { BeforeEach(func() { someConditionTime = metav1.Unix(1, 1) someCondition = RabbitmqClusterCondition{ - Type: RabbitmqClusterConditionType("a-type"), - Status: corev1.ConditionStatus("some-status"), + Type: "a-type", + Status: "some-status", LastTransitionTime: (*someConditionTime.DeepCopy()), Reason: "reasons", Message: "ship-it", @@ -29,7 +29,7 @@ var _ = Describe("Status", func() { }) It("changes the status and transition time", func() { - someCondition.UpdateState(corev1.ConditionStatus("maybe")) + someCondition.UpdateState("maybe") Expect(someCondition.Status).To(Equal(corev1.ConditionStatus("maybe"))) Expect(someCondition.LastTransitionTime).NotTo(Equal(someConditionTime)) @@ -38,7 +38,7 @@ var _ = Describe("Status", func() { }) It("preserves the status and transtion time", func() { - someCondition.UpdateState(corev1.ConditionStatus("some-status")) + someCondition.UpdateState("some-status") Expect(someCondition.Status).To(Equal(corev1.ConditionStatus("some-status"))) Expect(someCondition.LastTransitionTime).To(Equal(someConditionTime)) }) From 0e48cf4c8bc45f5f4ac30dc481c222e0f1a240e9 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Mon, 7 Sep 2020 19:07:43 +0100 Subject: [PATCH 5/6] Remove unused const and methods --- internal/metadata/metadata_suite_test.go | 9 --------- system_tests/system_tests.go | 5 +---- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/internal/metadata/metadata_suite_test.go b/internal/metadata/metadata_suite_test.go index b8e0e5c09..3ac1e660b 100644 --- a/internal/metadata/metadata_suite_test.go +++ b/internal/metadata/metadata_suite_test.go @@ -20,12 +20,3 @@ func TestResource(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Metadata Suite") } - -func testLabels(labels map[string]string) { - ExpectWithOffset(1, labels).To(SatisfyAll( - HaveKeyWithValue("foo", "bar"), - HaveKeyWithValue("rabbitmq", "is-great"), - HaveKeyWithValue("foo/app.kubernetes.io", "edgecase"), - Not(HaveKey("app.kubernetes.io/foo")), - )) -} diff --git a/system_tests/system_tests.go b/system_tests/system_tests.go index 6119eba2b..83f5a7345 100644 --- a/system_tests/system_tests.go +++ b/system_tests/system_tests.go @@ -23,10 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const ( - clientServiceSuffix = "client" - statefulSetSuffix = "server" -) +const statefulSetSuffix = "server" var _ = Describe("Operator", func() { var ( From 686e9ad0f54d85397569fd13048496113946f313 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Mon, 7 Sep 2020 19:08:37 +0100 Subject: [PATCH 6/6] Methods k8sCreateSecretTLS should use provided secretName variable --- system_tests/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system_tests/utils.go b/system_tests/utils.go index 918d0fa64..e25aa4b08 100644 --- a/system_tests/utils.go +++ b/system_tests/utils.go @@ -600,7 +600,7 @@ func createTLSSecret(secretName, secretNamespace, hostname string) string { // generate and write cert and key to file Expect(createCertificateChain(hostname, caCertFile, serverCertFile, serverKeyFile)).To(Succeed()) // create k8s tls secret - Expect(k8sCreateSecretTLS("rabbitmq-tls-test-secret", secretNamespace, serverCertPath, serverKeyPath)).To(Succeed()) + Expect(k8sCreateSecretTLS(secretName, secretNamespace, serverCertPath, serverKeyPath)).To(Succeed()) // remove server files Expect(os.Remove(serverKeyPath)).To(Succeed())