Skip to content

Commit

Permalink
Fix golangci-lint findings
Browse files Browse the repository at this point in the history
  • Loading branch information
timuthy committed Feb 26, 2021
1 parent c34e876 commit d74d510
Show file tree
Hide file tree
Showing 12 changed files with 68 additions and 66 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fmt:
# Check packages
.PHONY: check
check:
@$(REPO_ROOT)/vendor/github.com/gardener/gardener/hack/check.sh --golangci-lint-config=./.golangci.yaml . ./api/... ./pkg/... ./controllers/...
@$(REPO_ROOT)/vendor/github.com/gardener/gardener/hack/check.sh --golangci-lint-config=./.golangci.yaml ./api/... ./pkg/... ./controllers/...

# Generate code
.PHONY: generate
Expand Down
1 change: 1 addition & 0 deletions config/crd/bases/druid.gardener.cloud_etcds.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
Expand Down
112 changes: 58 additions & 54 deletions controllers/etcd_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"os"
"time"

"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/gardener/etcd-druid/pkg/common"
"github.com/gardener/gardener/pkg/utils/imagevector"
"github.com/ghodss/yaml"
Expand Down Expand Up @@ -50,23 +52,6 @@ const (
WithOwnerAnnotation StatefulSetInitializer = 2
)

const (
aws = "aws"
azure = "azure"
gcp = "gcp"
alicloud = "alicloud"
openstack = "openstack"
)

const (
s3 = "S3"
abs = "ABS"
gcs = "GCS"
oss = "OSS"
swift = "Swift"
local = "Local"
)

const (
timeout = time.Minute * 2
pollingInterval = time.Second * 2
Expand Down Expand Up @@ -168,23 +153,24 @@ var _ = Describe("Druid", func() {
Name: instance.Namespace,
},
}
c.Create(context.TODO(), &ns)
_, err = controllerutil.CreateOrUpdate(context.TODO(), c, &ns, func() error { return nil })
Expect(err).To(Not(HaveOccurred()))

storeSecret := instance.Spec.Backup.Store.SecretRef.Name
errors := createSecrets(c, instance.Namespace, storeSecret)
Expect(len(errors)).Should(BeZero())
})
It("should create and adopt statefulset", func() {
defer WithWd("..")()
err = c.Create(context.TODO(), instance)
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(context.TODO(), instance)).To(Succeed())
ss := appsv1.StatefulSet{}
Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, &ss) }, timeout, pollingInterval).Should(BeNil())
setStatefulSetReady(&ss)
err = c.Status().Update(context.TODO(), &ss)
Expect(err).NotTo(HaveOccurred())
})
AfterEach(func() {
c.Delete(context.TODO(), instance)
Expect(c.Delete(context.TODO(), instance)).To(Succeed())
})
})

Expand All @@ -193,6 +179,21 @@ var _ = Describe("Druid", func() {
var ss *appsv1.StatefulSet
instance := getEtcd(name, "default", false)
c := mgr.GetClient()
Expect(c.Create(context.TODO(), instance)).To(Succeed())
Eventually(func() error {
if err := c.Get(context.TODO(), client.ObjectKeyFromObject(instance), instance); err != nil {
return err
}
if instance.UID != "" {
instance.TypeMeta = metav1.TypeMeta{
APIVersion: "druid.gardener.cloud/v1alpha1",
Kind: "etcd",
}
return nil
}
return fmt.Errorf("etcd object not yet created")
}, timeout, pollingInterval).Should(BeNil())

switch setupStatefulSet {
case WithoutOwner:
ss = createStatefulset(name, "default", instance.Spec.Labels)
Expand All @@ -209,17 +210,13 @@ var _ = Describe("Druid", func() {
storeSecret := instance.Spec.Backup.Store.SecretRef.Name
errors := createSecrets(c, instance.Namespace, storeSecret)
Expect(len(errors)).Should(BeZero())
c.Create(context.TODO(), ss)
Expect(c.Create(context.TODO(), ss)).To(Succeed())
defer WithWd("..")()
err := c.Create(context.TODO(), instance)
Expect(err).NotTo(HaveOccurred())
s := &appsv1.StatefulSet{}
Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, s) }, timeout, pollingInterval).Should(BeNil())
setStatefulSetReady(s)
err = c.Status().Update(context.TODO(), s)
Expect(err).NotTo(HaveOccurred())
err = c.Delete(context.TODO(), instance)
Expect(err).NotTo(HaveOccurred())
Expect(c.Status().Update(context.TODO(), s)).To(Succeed())
Expect(c.Delete(context.TODO(), instance)).To(Succeed())
},
Entry("when statefulset not owned by etcd, druid should adopt the statefulset", "foo20", WithoutOwner),
Entry("when statefulset owned by etcd with owner reference set, druid should remove ownerref and add annotations", "foo21", WithOwnerReference),
Expand All @@ -240,10 +237,8 @@ var _ = Describe("Druid", func() {
c = mgr.GetClient()
p = createPod(fmt.Sprintf("%s-0", instance.Name), "default", instance.Spec.Labels)
ss = createStatefulset(instance.Name, instance.Namespace, instance.Spec.Labels)
err = c.Create(context.TODO(), p)
Expect(err).NotTo(HaveOccurred())
err = c.Create(context.TODO(), ss)
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(context.TODO(), p)).To(Succeed())
Expect(c.Create(context.TODO(), ss)).To(Succeed())
p.Status.ContainerStatuses = []corev1.ContainerStatus{
{
Name: "Container-0",
Expand All @@ -264,17 +259,15 @@ var _ = Describe("Druid", func() {
})
It("should restart pod", func() {
defer WithWd("..")()
err = c.Create(context.TODO(), instance)
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(context.TODO(), instance)).To(Succeed())
Eventually(func() error { return podDeleted(c, instance) }, timeout, pollingInterval).Should(BeNil())
})
AfterEach(func() {
s := &appsv1.StatefulSet{}
Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, s) }, timeout, pollingInterval).Should(BeNil())
setStatefulSetReady(s)
err = c.Status().Update(context.TODO(), s)
Expect(err).NotTo(HaveOccurred())
c.Delete(context.TODO(), instance)
Expect(c.Status().Update(context.TODO(), s)).To(Succeed())
Expect(c.Delete(context.TODO(), instance)).To(Succeed())
})
})
})
Expand All @@ -285,6 +278,21 @@ var _ = Describe("Druid", func() {
var sts appsv1.StatefulSet
instance := getEtcd(name, "default", false)
c := mgr.GetClient()
Expect(c.Create(context.TODO(), instance)).To(Succeed())
Eventually(func() error {
if err := c.Get(context.TODO(), client.ObjectKeyFromObject(instance), instance); err != nil {
return err
}
if instance.UID != "" {
instance.TypeMeta = metav1.TypeMeta{
APIVersion: "druid.gardener.cloud/v1alpha1",
Kind: "etcd",
}
return nil
}
return fmt.Errorf("etcd object not yet created")
}, timeout, pollingInterval).Should(BeNil())

switch setupStatefulSet {
case WithoutOwner:
ss = createStatefulset(name, "default", instance.Spec.Labels)
Expand All @@ -301,26 +309,23 @@ var _ = Describe("Druid", func() {
storeSecret := instance.Spec.Backup.Store.SecretRef.Name
errors := createSecrets(c, instance.Namespace, storeSecret)
Expect(len(errors)).Should(BeZero())
c.Create(context.TODO(), ss)
Expect(c.Create(context.TODO(), ss)).To(Succeed())
defer WithWd("..")()
err := c.Create(context.TODO(), instance)
Expect(err).NotTo(HaveOccurred())
Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, &sts) }, timeout, pollingInterval).Should(BeNil())
// This go-routine is to set that statefulset is ready manually as statefulset controller is absent for tests.
go func() {
for {
select {
case <-time.After(time.Second * 2):
setAndCheckStatefulSetReady(c, instance)
Expect(setAndCheckStatefulSetReady(c, instance)).To(Succeed())
case <-stopCh:
return
}
}
}()
Eventually(func() error { return isEtcdReady(c, instance) }, timeout, pollingInterval).Should(BeNil())
close(stopCh)
err = c.Delete(context.TODO(), instance)
Expect(err).NotTo(HaveOccurred())
Expect(c.Delete(context.TODO(), instance)).To(Succeed())
Eventually(func() error { return statefulSetRemoved(c, &sts) }, timeout, pollingInterval).Should(BeNil())
Eventually(func() error { return etcdRemoved(c, instance) }, timeout, pollingInterval).Should(BeNil())
},
Expand All @@ -346,7 +351,10 @@ var _ = Describe("Druid", func() {
Name: instance.Namespace,
},
}
c.Create(context.TODO(), &ns)

_, err = controllerutil.CreateOrUpdate(context.TODO(), c, &ns, func() error { return nil })
Expect(err).To(Not(HaveOccurred()))

if instance.Spec.Backup.Store != nil && instance.Spec.Backup.Store.SecretRef != nil {
storeSecret := instance.Spec.Backup.Store.SecretRef.Name
errors := createSecrets(c, instance.Namespace, storeSecret)
Expand Down Expand Up @@ -408,7 +416,7 @@ func validateEtcdWithDefaults(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *

// Validate Metrics MetricsLevel
Expect(instance.Spec.Etcd.Metrics).To(BeNil())
Expect(config).To(HaveKeyWithValue(metricsKey, fmt.Sprintf("%s", druidv1alpha1.Basic)))
Expect(config).To(HaveKeyWithValue(metricsKey, string(druidv1alpha1.Basic)))

// Validate DefragmentationSchedule *string
Expect(instance.Spec.Etcd.DefragmentationSchedule).To(BeNil())
Expand Down Expand Up @@ -754,7 +762,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi

// Validate Metrics MetricsLevel
Expect(instance.Spec.Etcd.Metrics).NotTo(BeNil())
Expect(config).To(HaveKeyWithValue(metricsKey, fmt.Sprintf("%s", *instance.Spec.Etcd.Metrics)))
Expect(config).To(HaveKeyWithValue(metricsKey, string(*instance.Spec.Etcd.Metrics)))

// Validate DefragmentationSchedule *string
Expect(instance.Spec.Etcd.DefragmentationSchedule).NotTo(BeNil())
Expand Down Expand Up @@ -795,7 +803,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi
Expect(config).To(MatchKeys(IgnoreExtras, Keys{
"name": Equal(fmt.Sprintf("etcd-%s", instance.UID[:6])),
"data-dir": Equal("/var/etcd/data/new.etcd"),
"metrics": Equal(fmt.Sprintf("%s", *instance.Spec.Etcd.Metrics)),
"metrics": Equal(string(*instance.Spec.Etcd.Metrics)),
"snapshot-count": Equal(float64(75000)),
"enable-v2": Equal(false),
"quota-backend-bytes": Equal(float64(instance.Spec.Etcd.Quota.Value())),
Expand Down Expand Up @@ -1031,7 +1039,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi
fmt.Sprintf("--defragmentation-schedule=%s", *instance.Spec.Etcd.DefragmentationSchedule): Equal(fmt.Sprintf("--defragmentation-schedule=%s", *instance.Spec.Etcd.DefragmentationSchedule)),
fmt.Sprintf("--schedule=%s", *instance.Spec.Backup.FullSnapshotSchedule): Equal(fmt.Sprintf("--schedule=%s", *instance.Spec.Backup.FullSnapshotSchedule)),
fmt.Sprintf("%s=%s", "--garbage-collection-policy", *instance.Spec.Backup.GarbageCollectionPolicy): Equal(fmt.Sprintf("%s=%s", "--garbage-collection-policy", *instance.Spec.Backup.GarbageCollectionPolicy)),
fmt.Sprintf("%s=%s", "--storage-provider", store): Equal(fmt.Sprintf("%s=%s", "--storage-provider", fmt.Sprintf("%s", store))),
fmt.Sprintf("%s=%s", "--storage-provider", store): Equal(fmt.Sprintf("%s=%s", "--storage-provider", store)),
fmt.Sprintf("%s=%s", "--store-prefix", instance.Spec.Backup.Store.Prefix): Equal(fmt.Sprintf("%s=%s", "--store-prefix", instance.Spec.Backup.Store.Prefix)),
fmt.Sprintf("--delta-snapshot-memory-limit=%d", instance.Spec.Backup.DeltaSnapshotMemoryLimit.Value()): Equal(fmt.Sprintf("--delta-snapshot-memory-limit=%d", instance.Spec.Backup.DeltaSnapshotMemoryLimit.Value())),
fmt.Sprintf("--garbage-collection-policy=%s", *instance.Spec.Backup.GarbageCollectionPolicy): Equal(fmt.Sprintf("--garbage-collection-policy=%s", *instance.Spec.Backup.GarbageCollectionPolicy)),
Expand All @@ -1048,7 +1056,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi
ContainerPort: backupPort,
},
}),
"Image": Equal(fmt.Sprintf("%s", *instance.Spec.Backup.Image)),
"Image": Equal(*instance.Spec.Backup.Image),
"ImagePullPolicy": Equal(corev1.PullIfNotPresent),
"VolumeMounts": MatchElements(volumeMountIterator, IgnoreExtras, Elements{
*instance.Spec.VolumeClaimTemplate: MatchFields(IgnoreExtras, Fields{
Expand Down Expand Up @@ -1542,7 +1550,7 @@ func statefulsetIsCorrectlyReconciled(c client.Client, instance *druidv1alpha1.E
ctx, cancel := context.WithTimeout(context.TODO(), timeout)
defer cancel()
req := types.NamespacedName{
Name: fmt.Sprintf("%s", instance.Name),
Name: instance.Name,
Namespace: instance.Namespace,
}

Expand Down Expand Up @@ -1756,10 +1764,6 @@ func getEtcdWithTLS(name, namespace string) *druidv1alpha1.Etcd {
return getEtcd(name, namespace, true)
}

func getEtcdWithoutTLS(name, namespace string) *druidv1alpha1.Etcd {
return getEtcd(name, namespace, false)
}

func getEtcdWithDefault(name, namespace string) *druidv1alpha1.Etcd {
instance := &druidv1alpha1.Etcd{
ObjectMeta: metav1.ObjectMeta{
Expand Down
8 changes: 5 additions & 3 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"path/filepath"
"sync"
"testing"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -50,8 +51,6 @@ var (
k8sClient client.Client
testEnv *envtest.Environment
mgr manager.Manager
recFn reconcile.Reconciler
requests chan reconcile.Request
mgrStopped *sync.WaitGroup

testLog = ctrl.Log.WithName("test")
Expand Down Expand Up @@ -119,11 +118,14 @@ var _ = AfterSuite(func() {

func startTestManager(ctx context.Context, mgr manager.Manager) *sync.WaitGroup {
wg := &sync.WaitGroup{}
wg.Add(1)
go func() {
wg.Add(1)
Expect(mgr.Start(ctx)).NotTo(HaveOccurred())
wg.Done()
}()
syncCtx, syncCancel := context.WithTimeout(ctx, 1*time.Minute)
defer syncCancel()
mgr.GetCache().WaitForCacheSync(syncCtx)
return wg
}

Expand Down
5 changes: 3 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/gardener/etcd-druid/controllers"

"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
schemev1 "k8s.io/client-go/kubernetes/scheme"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -36,8 +37,8 @@ var (
)

func init() {
schemev1.AddToScheme(scheme)
druidv1alpha1.AddToScheme(scheme)
utilruntime.Must(schemev1.AddToScheme(scheme))
utilruntime.Must(druidv1alpha1.AddToScheme(scheme))

// +kubebuilder:scaffold:scheme
}
Expand Down
6 changes: 0 additions & 6 deletions pkg/predicate/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package predicate

import (
druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/runtime"

"sigs.k8s.io/controller-runtime/pkg/event"
Expand All @@ -26,11 +25,6 @@ import (
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
)

// Log is the logger for predicates.
var (
logger = logrus.New()
)

type or struct {
predicates []predicate.Predicate
}
Expand Down

0 comments on commit d74d510

Please sign in to comment.