Merge pull request #73 from gardener/fix-handler-update-race

alicloud: avoid provider update race if get zones fails

pkg/controller/provider/alicloud/handler.go

@@ -23,6 +23,7 @@ import (
 
  "github.com/gardener/external-dns-management/pkg/dns"
  "github.com/gardener/external-dns-management/pkg/dns/provider"
+ perrs "github.com/gardener/external-dns-management/pkg/dns/provider/errors"
  "github.com/gardener/external-dns-management/pkg/dns/provider/raw"
 )
 
@@ -54,7 +55,7 @@ func NewHandler(c *provider.DNSHandlerConfig) (provider.DNSHandler, error) {
 
  access, err := NewAccess(accessKeyID, accessKeySecret, c.Metrics)
  if err != nil {
- return nil, err
+ return nil, perrs.WrapAsHandlerError(err, "Creating alicloud access with client credentials failed")
  }
 
  h.access = access
@@ -84,7 +85,7 @@ func (h *Handler) getZones(cache provider.ZoneCache) (provider.DNSHostedZones, e
  }
  err := h.access.ListDomains(f)
  if err != nil {
- return nil, err
+ return nil, perrs.WrapAsHandlerError(err, "list domains failed")
  }
  }
 
@@ -108,7 +109,7 @@ func (h *Handler) getZones(cache provider.ZoneCache) (provider.DNSHostedZones, e
  // As a result, h domain should not be appended to the hosted zones
  continue
  }
- return nil, err
+ return nil, perrs.WrapAsHandlerError(err, "list records failed")
  }
  hostedZone := provider.NewDNSHostedZone(h.ProviderType(), z.DomainId, z.DomainName, z.DomainName, forwarded, false)
  zones = append(zones, hostedZone)
@@ -133,7 +134,7 @@ func (h *Handler) getZoneState(zone provider.DNSHostedZone, cache provider.ZoneC
  }
  err := h.access.ListRecords(zone.Key(), f)
  if err != nil {
- return nil, err
+ return nil, perrs.WrapAsHandlerError(err, "list records failed")
  }
  state.CalculateDNSSets()
  return state, nil

pkg/controller/provider/azure/handler.go

@@ -24,13 +24,13 @@ import (
  "strings"
 
  "github.com/gardener/controller-manager-library/pkg/logger"
- "github.com/pkg/errors"
 
  azure "github.com/Azure/azure-sdk-for-go/services/dns/mgmt/2018-05-01/dns"
  "github.com/Azure/go-autorest/autorest/azure/auth"
 
  "github.com/gardener/external-dns-management/pkg/dns"
  "github.com/gardener/external-dns-management/pkg/dns/provider"
+ perrs "github.com/gardener/external-dns-management/pkg/dns/provider/errors"
 )
 
 type Handler struct {
@@ -72,7 +72,7 @@ func NewHandler(c *provider.DNSHandlerConfig) (provider.DNSHandler, error) {
 
  authorizer, err := auth.NewClientCredentialsConfig(clientID, clientSecret, tenantID).Authorizer()
  if err != nil {
- return nil, errors.Wrap(err, "Creating Azure authorizer with client credentials failed")
+ return nil, perrs.WrapAsHandlerError(err, "Creating Azure authorizer with client credentials failed")
  }
 
  zonesClient := azure.NewZonesClient(subscriptionID)
@@ -86,7 +86,7 @@ func NewHandler(c *provider.DNSHandlerConfig) (provider.DNSHandler, error) {
  ctx := context.TODO()
  _, err = zonesClient.List(ctx, &one)
  if err != nil {
- return nil, errors.Wrap(err, "Authentication test to Azure with client credentials failed. Please check secret for DNSProvider.")
+ return nil, perrs.WrapAsHandlerError(err, "Authentication test to Azure with client credentials failed. Please check secret for DNSProvider.")
  }
 
  h.zonesClient = &zonesClient
@@ -115,7 +115,7 @@ func (h *Handler) getZones(cache provider.ZoneCache) (provider.DNSHostedZones, e
  results, err := h.zonesClient.ListComplete(h.ctx, nil)
  h.config.Metrics.AddRequests("ZonesClient_ListComplete", 1)
  if err != nil {
- return nil, errors.Wrap(err, "Listing DNS zones failed")
+ return nil, perrs.WrapAsHandlerError(err, "Listing DNS zones failed")
  }
 
  for ; results.NotDone(); results.Next() {
@@ -179,7 +179,7 @@ func (h *Handler) getZoneState(zone provider.DNSHostedZone, cache provider.ZoneC
  results, err := h.recordsClient.ListAllByDNSZoneComplete(h.ctx, resourceGroup, zoneName, nil, "")
  h.config.Metrics.AddRequests("RecordSetsClient_ListAllByDNSZoneComplete", 1)
  if err != nil {
- return nil, errors.Wrapf(err, "Listing DNS zone state for zone %s failed", zoneName)
+ return nil, perrs.WrapfAsHandlerError(err, "Listing DNS zone state for zone %s failed", zoneName)
  }
 
  for ; results.NotDone(); results.Next() {

pkg/controller/provider/cloudflare/state.go

@@ -34,6 +34,6 @@ func (r *Record) GetValue() string {
  }
  return r.Content
 }
-func (r *Record) GetTTL() int  { return r.TTL }
-func (r *Record) SetTTL(ttl int)  { r.TTL = ttl }
-func (r *Record) Copy() raw.Record  { n := *r; return &n }
+func (r *Record) GetTTL() int { return r.TTL }
+func (r *Record) SetTTL(ttl int) { r.TTL = ttl }
+func (r *Record) Copy() raw.Record { n := *r; return &n }

pkg/dns/provider/errors/handlererror.go

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2020 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file
+ *
+ * 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
+ *
+ */
+
+package errors
+
+import (
+ pkgerrors "github.com/pkg/errors"
+)
+
+type handlerError struct {
+ err error
+}
+
+func WrapAsHandlerError(err error, msg string) error {
+ return pkgerrors.Wrap(&handlerError{err: err}, msg)
+}
+
+func WrapfAsHandlerError(err error, msg string, args ...interface{}) error {
+ return pkgerrors.Wrapf(&handlerError{err: err}, msg, args...)
+}
+
+func IsHandlerError(err error) bool {
+ _, ok := err.(*handlerError)
+ return ok
+}
+
+func (e *handlerError) Error() string {
+ return e.err.Error()
+}
+
+func (e *handlerError) Cause() error {
+ return e.err
+}

pkg/dns/provider/provider.go

@@ -25,6 +25,9 @@ import (
  "sync"
  "time"
 
+ pkgerrors "github.com/pkg/errors"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/apimachinery/pkg/api/errors"
  "k8s.io/apimachinery/pkg/runtime"
 
  "github.com/gardener/external-dns-management/pkg/server/metrics"
@@ -36,9 +39,6 @@ import (
  "github.com/gardener/controller-manager-library/pkg/logger"
  "github.com/gardener/controller-manager-library/pkg/resources"
  "github.com/gardener/controller-manager-library/pkg/utils"
-
- corev1 "k8s.io/api/core/v1"
- "k8s.io/apimachinery/pkg/api/errors"
 )
 
 const ZoneCachePrefix = "zc-"
@@ -323,7 +323,7 @@ func updateDNSProvider(logger logger.LogContext, state *state, provider *dnsutil
  zones, err := this.account.GetZones()
  if err != nil {
  this.zones = nil
- return this, this.failed(logger, false, fmt.Errorf("cannot get zones: %s", err), true)
+ return this, this.failed(logger, false, pkgerrors.Wrap(err, "cannot get zones"), true)
  }
 
  zinc, zexc := prepareSelection(provider.DNSProvider().Spec.Zones)

pkg/dns/utils/utils_provider.go

@@ -26,6 +26,7 @@ import (
  "github.com/gardener/controller-manager-library/pkg/utils"
 
  api "github.com/gardener/external-dns-management/pkg/apis/dns/v1alpha1"
+ "github.com/gardener/external-dns-management/pkg/dns/provider/errors"
 )
 
 var DNSProviderType = (*api.DNSProvider)(nil)
@@ -60,11 +61,26 @@ func (this *DNSProviderObject) SetStateWithError(state string, err error) bool {
  }
 
  message := err.Error()
- if cause, ok := err.(causer); ok {
- suffix := cause.Error()
+ handlerErrorMsg := ""
+ for {
+ if cerr, ok := err.(causer); ok {
+ cause := cerr.Cause()
+ if cause == nil || cause == err {
+ break
+ }
+ if errors.IsHandlerError(cause) {
+ handlerErrorMsg = cause.Error()
+ break
+ }
+ err = cause
+ } else {
+ break
+ }
+ }
+ if handlerErrorMsg != "" {
  prefix := message
- if strings.HasSuffix(message, suffix) {
- prefix = message[:len(message)-len(suffix)]
+ if len(message) > len(handlerErrorMsg) && strings.HasSuffix(message, handlerErrorMsg) {
+ prefix = message[:len(message)-len(handlerErrorMsg)]
  }
  return this.SetState(api.STATE_ERROR, message, prefix)
  }