From 5d3e475e29ac4c61a4a6aff68ba3e0c0d2d78dcc Mon Sep 17 00:00:00 2001 From: Rasel Hossain <59439107+Superm4n97@users.noreply.github.com> Date: Tue, 17 Jan 2023 22:01:41 +0600 Subject: [PATCH] Fix reconciler error (#18) Signed-off-by: rasel --- pkg/cmds/run.go | 8 +- .../external-dns/externaldns_controller.go | 89 ++++++++++++++----- pkg/controllers/external-dns/suite_test.go | 4 +- pkg/credentials/aws.go | 4 +- pkg/credentials/azure.go | 4 +- pkg/credentials/cloudflare.go | 4 +- pkg/credentials/google.go | 4 +- pkg/credentials/secret.go | 14 +-- pkg/informers/dynamicWatcher.go | 6 +- pkg/plan/plan.go | 14 +-- 10 files changed, 98 insertions(+), 53 deletions(-) diff --git a/pkg/cmds/run.go b/pkg/cmds/run.go index 3b7baf78..c6b58851 100644 --- a/pkg/cmds/run.go +++ b/pkg/cmds/run.go @@ -19,8 +19,8 @@ package cmds import ( "os" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" - externaldnscontrollers "kubeops.dev/external-dns-operator/pkg/controllers/external-dns" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + controllers "kubeops.dev/external-dns-operator/pkg/controllers/external-dns" "github.com/spf13/cobra" v "gomodules.xyz/x/version" @@ -45,7 +45,7 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(externaldnsv1alpha1.AddToScheme(scheme)) + utilruntime.Must(api.AddToScheme(scheme)) } func NewCmdRun() *cobra.Command { @@ -94,7 +94,7 @@ func NewCmdRun() *cobra.Command { os.Exit(1) } - if err = (&externaldnscontrollers.ExternalDNSReconciler{ + if err = (&controllers.ExternalDNSReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { diff --git a/pkg/controllers/external-dns/externaldns_controller.go b/pkg/controllers/external-dns/externaldns_controller.go index e02a8112..4802a448 100644 --- a/pkg/controllers/external-dns/externaldns_controller.go +++ b/pkg/controllers/external-dns/externaldns_controller.go @@ -20,11 +20,12 @@ import ( "context" "sync" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" "kubeops.dev/external-dns-operator/pkg/credentials" "kubeops.dev/external-dns-operator/pkg/informers" "kubeops.dev/external-dns-operator/pkg/plan" + "github.com/pkg/errors" core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2" @@ -48,19 +49,19 @@ type ExternalDNSReconciler struct { watcher *informers.ObjectTracker } -func newConditionPtr(reason string, message string, generation int64, conditionStatus bool) *kmapi.Condition { +func newCondition(reason string, message string, generation int64, conditionStatus bool) *kmapi.Condition { newCondition := kmapi.NewCondition(reason, message, generation, conditionStatus) return &newCondition } -func phasePointer(phase externaldnsv1alpha1.ExternalDNSPhase) *externaldnsv1alpha1.ExternalDNSPhase { +func newPhase(phase api.ExternalDNSPhase) *api.ExternalDNSPhase { return &phase } // update the status of the crd, conditionType is the reason of the condition -func (r *ExternalDNSReconciler) updateEdnsStatus(ctx context.Context, edns *externaldnsv1alpha1.ExternalDNS, newCondition *kmapi.Condition, phase *externaldnsv1alpha1.ExternalDNSPhase) error { +func (r *ExternalDNSReconciler) updateEdnsStatus(ctx context.Context, edns *api.ExternalDNS, newCondition *kmapi.Condition, phase *api.ExternalDNSPhase) error { _, _, patchErr := kmc.PatchStatus(ctx, r.Client, edns, func(obj client.Object) client.Object { - in := obj.(*externaldnsv1alpha1.ExternalDNS) + in := obj.(*api.ExternalDNS) if phase != nil { in.Status.Phase = *phase } @@ -72,9 +73,9 @@ func (r *ExternalDNSReconciler) updateEdnsStatus(ctx context.Context, edns *exte return patchErr } -func (r ExternalDNSReconciler) patchDNSRecords(ctx context.Context, edns *externaldnsv1alpha1.ExternalDNS, dnsRecs []externaldnsv1alpha1.DNSRecord) error { +func (r ExternalDNSReconciler) patchDNSRecords(ctx context.Context, edns *api.ExternalDNS, dnsRecs []api.DNSRecord) error { _, _, patchErr := kmc.PatchStatus(ctx, r.Client, edns, func(obj client.Object) client.Object { - in := obj.(*externaldnsv1alpha1.ExternalDNS) + in := obj.(*api.ExternalDNS) in.Status.DNSRecords = dnsRecs return in }) @@ -87,7 +88,7 @@ func (r *ExternalDNSReconciler) Reconcile(ctx context.Context, req ctrl.Request) // GET EXTERNAL DNS ednsKey := req.NamespacedName - edns := &externaldnsv1alpha1.ExternalDNS{} + edns := &api.ExternalDNS{} if err := r.Get(ctx, ednsKey, edns); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) @@ -95,17 +96,35 @@ func (r *ExternalDNSReconciler) Reconcile(ctx context.Context, req ctrl.Request) edns = edns.DeepCopy() if edns.Status.Phase == "" { - if patchErr := r.updateEdnsStatus(ctx, edns, nil, phasePointer(externaldnsv1alpha1.ExternalDNSPhaseInProgress)); patchErr != nil { + if patchErr := r.updateEdnsStatus( + ctx, + edns, + nil, + newPhase(api.ExternalDNSPhaseInProgress), + ); patchErr != nil { return ctrl.Result{}, patchErr } } // REGISTER WATCHER if err := informers.RegisterWatcher(ctx, edns, r.watcher, r.Client); err != nil { - return ctrl.Result{}, r.updateEdnsStatus(ctx, edns, newConditionPtr(externaldnsv1alpha1.CreateAndRegisterWatcher, err.Error(), edns.Generation, false), phasePointer(externaldnsv1alpha1.ExternalDNSPhaseFailed)) + if patchErr := r.updateEdnsStatus( + ctx, + edns, + newCondition(api.CreateAndRegisterWatcher, err.Error(), edns.Generation, false), + newPhase(api.ExternalDNSPhaseFailed), + ); patchErr != nil { + err = errors.Wrap(err, patchErr.Error()) + } + return ctrl.Result{}, err } - if patchErr := r.updateEdnsStatus(ctx, edns, newConditionPtr(externaldnsv1alpha1.CreateAndRegisterWatcher, "Watcher registered", edns.Generation, true), nil); patchErr != nil { + if patchErr := r.updateEdnsStatus( + ctx, + edns, + newCondition(api.CreateAndRegisterWatcher, "Watcher registered", edns.Generation, true), + nil, + ); patchErr != nil { return ctrl.Result{}, patchErr } @@ -116,10 +135,23 @@ func (r *ExternalDNSReconciler) Reconcile(ctx context.Context, req ctrl.Request) // create and set provider secret credentials and environment variables err := credentials.SetCredential(ctx, r.Client, edns) if err != nil { - return ctrl.Result{}, r.updateEdnsStatus(ctx, edns, newConditionPtr(externaldnsv1alpha1.GetProviderSecret, err.Error(), edns.Generation, false), phasePointer(externaldnsv1alpha1.ExternalDNSPhaseFailed)) + if patchErr := r.updateEdnsStatus( + ctx, + edns, + newCondition(api.GetProviderSecret, err.Error(), edns.Generation, false), + newPhase(api.ExternalDNSPhaseFailed), + ); patchErr != nil { + err = errors.Wrap(err, patchErr.Error()) + } + return ctrl.Result{}, err } - if patchErr := r.updateEdnsStatus(ctx, edns, newConditionPtr(externaldnsv1alpha1.GetProviderSecret, "Provider credential configured", edns.Generation, true), nil); patchErr != nil { + if patchErr := r.updateEdnsStatus( + ctx, + edns, + newCondition(api.GetProviderSecret, "Provider credential configured", edns.Generation, true), + nil, + ); patchErr != nil { return ctrl.Result{}, patchErr } @@ -128,7 +160,15 @@ func (r *ExternalDNSReconciler) Reconcile(ctx context.Context, req ctrl.Request) // successMsg is used to identify whether the 'plan applied' or 'already up to date' dnsRecs, err := plan.SetDNSRecords(ctx, edns) if err != nil { - return ctrl.Result{}, r.updateEdnsStatus(ctx, edns, newConditionPtr(externaldnsv1alpha1.CreateAndApplyPlan, err.Error(), edns.Generation, false), phasePointer(externaldnsv1alpha1.ExternalDNSPhaseFailed)) + if patchErr := r.updateEdnsStatus( + ctx, + edns, + newCondition(api.CreateAndApplyPlan, err.Error(), edns.Generation, false), + newPhase(api.ExternalDNSPhaseFailed), + ); patchErr != nil { + err = errors.Wrap(err, patchErr.Error()) + } + return ctrl.Result{}, err } err = r.patchDNSRecords(ctx, edns, dnsRecs) @@ -136,7 +176,12 @@ func (r *ExternalDNSReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - return ctrl.Result{}, r.updateEdnsStatus(ctx, edns, newConditionPtr(externaldnsv1alpha1.CreateAndApplyPlan, "plan applied", edns.Generation, true), phasePointer(externaldnsv1alpha1.ExternalDNSPhaseCurrent)) + return ctrl.Result{}, r.updateEdnsStatus( + ctx, + edns, + newCondition(api.CreateAndApplyPlan, "plan applied", edns.Generation, true), + newPhase(api.ExternalDNSPhaseCurrent), + ) } // SetupWithManager sets up the controller with the Manager. @@ -144,30 +189,30 @@ func (r *ExternalDNSReconciler) SetupWithManager(mgr ctrl.Manager) error { secretToEdns := handler.EnqueueRequestsFromMapFunc(func(object client.Object) []reconcile.Request { reconcileReq := make([]reconcile.Request, 0) ctx := context.TODO() - ednsList := &externaldnsv1alpha1.ExternalDNSList{} + ednsList := &api.ExternalDNSList{} if err := mgr.GetClient().List(ctx, ednsList, client.InNamespace(object.GetNamespace())); err != nil { return reconcileReq } for _, edns := range ednsList.Items { - switch edns.Spec.Provider.String() { - case externaldnsv1alpha1.ProviderAWS.String(): + switch edns.Spec.Provider { + case api.ProviderAWS: if edns.Spec.AWS != nil && edns.Spec.AWS.SecretRef != nil && edns.Spec.AWS.SecretRef.Name == object.GetName() { reconcileReq = append(reconcileReq, reconcile.Request{NamespacedName: client.ObjectKey{Name: edns.Name, Namespace: edns.Namespace}}) } - case externaldnsv1alpha1.ProviderAzure.String(): + case api.ProviderAzure: if edns.Spec.Azure != nil && edns.Spec.Azure.SecretRef != nil && edns.Spec.Azure.SecretRef.Name == object.GetName() { reconcileReq = append(reconcileReq, reconcile.Request{NamespacedName: client.ObjectKey{Name: edns.Name, Namespace: edns.Namespace}}) } - case externaldnsv1alpha1.ProviderGoogle.String(): + case api.ProviderGoogle: if edns.Spec.Google != nil && edns.Spec.Google.SecretRef != nil && edns.Spec.Google.SecretRef.Name == object.GetName() { reconcileReq = append(reconcileReq, reconcile.Request{NamespacedName: client.ObjectKey{Name: edns.Name, Namespace: edns.Namespace}}) } - case externaldnsv1alpha1.ProviderCloudflare.String(): + case api.ProviderCloudflare: if edns.Spec.Cloudflare != nil && edns.Spec.Cloudflare.SecretRef != nil && edns.Spec.Cloudflare.SecretRef.Name == object.GetName() { reconcileReq = append(reconcileReq, reconcile.Request{NamespacedName: client.ObjectKey{Name: edns.Name, Namespace: edns.Namespace}}) } @@ -179,7 +224,7 @@ func (r *ExternalDNSReconciler) SetupWithManager(mgr ctrl.Manager) error { // for dynamic watcher controller, err := ctrl.NewControllerManagedBy(mgr). - For(&externaldnsv1alpha1.ExternalDNS{}). + For(&api.ExternalDNS{}). Watches(&source.Kind{Type: &core.Secret{}}, secretToEdns). Build(r) if err != nil { diff --git a/pkg/controllers/external-dns/suite_test.go b/pkg/controllers/external-dns/suite_test.go index e96f2438..c5f1fff1 100644 --- a/pkg/controllers/external-dns/suite_test.go +++ b/pkg/controllers/external-dns/suite_test.go @@ -22,7 +22,7 @@ import ( //+kubebuilder:scaffold:imports - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -67,7 +67,7 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(cfg).NotTo(BeNil()) - err = externaldnsv1alpha1.AddToScheme(scheme.Scheme) + err = api.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:scheme diff --git a/pkg/credentials/aws.go b/pkg/credentials/aws.go index 77f4a07c..3912b21f 100644 --- a/pkg/credentials/aws.go +++ b/pkg/credentials/aws.go @@ -22,7 +22,7 @@ import ( "fmt" "os" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -36,7 +36,7 @@ func validAWSSecret(secret *core.Secret, key string) bool { return found } -func setAWSCredential(ctx context.Context, kc client.Client, edns *externaldnsv1alpha1.ExternalDNS) error { +func setAWSCredential(ctx context.Context, kc client.Client, edns *api.ExternalDNS) error { if err := resetEnvVariables(AWSSharedCredentialsFile); err != nil { return err } diff --git a/pkg/credentials/azure.go b/pkg/credentials/azure.go index 3e48d6aa..fa9bf181 100644 --- a/pkg/credentials/azure.go +++ b/pkg/credentials/azure.go @@ -22,7 +22,7 @@ import ( "fmt" "os" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -34,7 +34,7 @@ func validAzureSecret(secret *core.Secret, key string) bool { return found } -func setAzureCredential(ctx context.Context, kc client.Client, edns *externaldnsv1alpha1.ExternalDNS) error { +func setAzureCredential(ctx context.Context, kc client.Client, edns *api.ExternalDNS) error { // for azure, user must have to provide ProviderSecretRef if edns.Spec.Azure == nil || edns.Spec.Azure.SecretRef == nil { return errors.New("providerSecretRef is not given for azure provider") diff --git a/pkg/credentials/cloudflare.go b/pkg/credentials/cloudflare.go index 0420c78a..00972e73 100644 --- a/pkg/credentials/cloudflare.go +++ b/pkg/credentials/cloudflare.go @@ -21,7 +21,7 @@ import ( "errors" "os" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -46,7 +46,7 @@ func validCFSecret(secret *core.Secret, tokenKey, apiKey, apiEmail string) bool } } -func setCloudflareCredentials(ctx context.Context, kc client.Client, edns *externaldnsv1alpha1.ExternalDNS) error { +func setCloudflareCredentials(ctx context.Context, kc client.Client, edns *api.ExternalDNS) error { if err := resetEnvVariables(CFApiToken, CFApiKey, CFApiEmail, CFBaseURL); err != nil { return err } diff --git a/pkg/credentials/google.go b/pkg/credentials/google.go index 6de1eecf..f3638922 100644 --- a/pkg/credentials/google.go +++ b/pkg/credentials/google.go @@ -22,7 +22,7 @@ import ( "fmt" "os" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -36,7 +36,7 @@ func validGoogleSecret(secret *core.Secret, key string) bool { return found } -func setGoogleCredential(ctx context.Context, kc client.Client, edns *externaldnsv1alpha1.ExternalDNS) error { +func setGoogleCredential(ctx context.Context, kc client.Client, edns *api.ExternalDNS) error { if err := resetEnvVariables(GoogleApplicationCredentials); err != nil { return err } diff --git a/pkg/credentials/secret.go b/pkg/credentials/secret.go index 64a4d37c..c73b012d 100644 --- a/pkg/credentials/secret.go +++ b/pkg/credentials/secret.go @@ -21,7 +21,7 @@ import ( "errors" "os" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -46,18 +46,18 @@ func resetEnvVariables(list ...string) error { return nil } -func SetCredential(ctx context.Context, kc client.Client, edns *externaldnsv1alpha1.ExternalDNS) error { - switch edns.Spec.Provider.String() { - case externaldnsv1alpha1.ProviderAWS.String(): +func SetCredential(ctx context.Context, kc client.Client, edns *api.ExternalDNS) error { + switch edns.Spec.Provider { + case api.ProviderAWS: return setAWSCredential(ctx, kc, edns) - case externaldnsv1alpha1.ProviderCloudflare.String(): + case api.ProviderCloudflare: return setCloudflareCredentials(ctx, kc, edns) - case externaldnsv1alpha1.ProviderAzure.String(): + case api.ProviderAzure: return setAzureCredential(ctx, kc, edns) - case externaldnsv1alpha1.ProviderGoogle.String(): + case api.ProviderGoogle: return setGoogleCredential(ctx, kc, edns) default: diff --git a/pkg/informers/dynamicWatcher.go b/pkg/informers/dynamicWatcher.go index 8755b41c..e4434528 100644 --- a/pkg/informers/dynamicWatcher.go +++ b/pkg/informers/dynamicWatcher.go @@ -21,7 +21,7 @@ import ( "reflect" "sync" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -108,11 +108,11 @@ func getRuntimeObject(gvk schema.GroupVersionKind) runtime.Object { return unObj } -func RegisterWatcher(ctx context.Context, crd *externaldnsv1alpha1.ExternalDNS, watcher *ObjectTracker, r client.Client) error { +func RegisterWatcher(ctx context.Context, crd *api.ExternalDNS, watcher *ObjectTracker, r client.Client) error { sourceHandler := func(object client.Object) []reconcile.Request { reconcileReq := make([]reconcile.Request, 0) - dnsList := &externaldnsv1alpha1.ExternalDNSList{} + dnsList := &api.ExternalDNSList{} if err := r.List(ctx, dnsList); err != nil { klog.Errorf("failed to list the external dns resources: %s", err.Error()) diff --git a/pkg/plan/plan.go b/pkg/plan/plan.go index 40e2fc78..fe1ec89b 100644 --- a/pkg/plan/plan.go +++ b/pkg/plan/plan.go @@ -24,7 +24,7 @@ import ( "strings" "time" - externaldnsv1alpha1 "kubeops.dev/external-dns-operator/apis/external/v1alpha1" + api "kubeops.dev/external-dns-operator/apis/external/v1alpha1" "github.com/sirupsen/logrus" "gomodules.xyz/sets" @@ -206,7 +206,7 @@ var defaultConfig = externaldns.Config{ IBMCloudConfigFile: "/etc/kubernetes/ibmcloud.json", } -func SetDNSRecords(ctx context.Context, edns *externaldnsv1alpha1.ExternalDNS) ([]externaldnsv1alpha1.DNSRecord, error) { +func SetDNSRecords(ctx context.Context, edns *api.ExternalDNS) ([]api.DNSRecord, error) { cfg := convertEDNSObjectToCfg(edns) endpointsSource, err := createEndpointsSource(ctx, cfg) @@ -237,7 +237,7 @@ func SetDNSRecords(ctx context.Context, edns *externaldnsv1alpha1.ExternalDNS) ( } // create and apply dns plan, If plan is successfully applied then returns dns record, which defines the desired records of the plan -func createAndApplyPlan(ctx context.Context, cfg *externaldns.Config, r registry.Registry, endpointSource source.Source) ([]externaldnsv1alpha1.DNSRecord, error) { +func createAndApplyPlan(ctx context.Context, cfg *externaldns.Config, r registry.Registry, endpointSource source.Source) ([]api.DNSRecord, error) { var domainFilter endpoint.DomainFilter if cfg.RegexDomainFilter.String() != "" { domainFilter = endpoint.NewRegexDomainFilter(cfg.RegexDomainFilter, cfg.RegexDomainExclusion) @@ -292,7 +292,7 @@ func createAndApplyPlan(ctx context.Context, cfg *externaldns.Config, r registry klog.Info("Desired: ", pln.Desired) klog.Info("Current: ", pln.Current) - dnsRecs := make([]externaldnsv1alpha1.DNSRecord, 0) + dnsRecs := make([]api.DNSRecord, 0) if pln.Changes.HasChanges() { err = r.ApplyChanges(ctx, pln.Changes) @@ -314,13 +314,13 @@ func createAndApplyPlan(ctx context.Context, cfg *externaldns.Config, r registry for _, rec := range pln.Desired { if managedRecordsTypes.Has(rec.RecordType) { - dnsRecs = append(dnsRecs, externaldnsv1alpha1.DNSRecord{Name: rec.DNSName, Target: rec.Targets.String()}) + dnsRecs = append(dnsRecs, api.DNSRecord{Name: rec.DNSName, Target: rec.Targets.String()}) } } return dnsRecs, nil } -func convertEDNSObjectToCfg(edns *externaldnsv1alpha1.ExternalDNS) *externaldns.Config { +func convertEDNSObjectToCfg(edns *api.ExternalDNS) *externaldns.Config { config := defaultConfig if edns.Namespace != "" { @@ -490,7 +490,7 @@ func convertEDNSObjectToCfg(edns *externaldnsv1alpha1.ExternalDNS) *externaldns. } // for azure provide - if edns.Spec.Provider.String() == externaldnsv1alpha1.ProviderAzure.String() { + if edns.Spec.Provider == api.ProviderAzure { // hard-code assignment of AzureConfigFile path config.AzureConfigFile = fmt.Sprintf("/tmp/%s-%s-credential", edns.Namespace, edns.Name) }