Skip to content

Commit

Permalink
Give better error if config entry already created (#420)
Browse files Browse the repository at this point in the history
Previously if the config entry already existed in Consul we'd give an
error like:

  config entry managed in different datacenter: "other-dc"

This worked well if the config entry had been created by a controller
running in another Kube cluster, but if the user had created the config
entry directly in Consul then it wouldn't have the datacenter annotation
and so the error would actually read:

  config entry managed in different datacenter: ""

Which was confusing.

This change changes the error in that case to read:

  config entry already exists in Consul
  • Loading branch information
lkysow authored Jan 26, 2021
1 parent 74964a4 commit 3b85413
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 295 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ FEATURES:
* CRDs: support annotation `consul.hashicorp.com/migrate-entry` on custom resources
that will allow an existing config entry to be migrated onto a Kubernetes custom resource. [[GH-419](https://github.com/hashicorp/consul-k8s/pull/419)]

IMPROVEMENTS:
* CRDs: give a more descriptive error when a config entry already exists in Consul. [[GH-420](https://github.com/hashicorp/consul-k8s/pull/420)]

## 0.23.0 (January 22, 2021)

BUG FIXES:
Expand Down
22 changes: 19 additions & 3 deletions controller/configentry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,19 +190,22 @@ func (r *ConfigEntryController) ReconcileEntry(
return r.syncFailed(ctx, logger, crdCtrl, configEntry, ConsulAgentError, err)
}

requiresMigration := false
sourceDatacenter := entry.GetMeta()[common.DatacenterKey]

// Check if the config entry is managed by our datacenter.
// Do not process resource if the entry was not created within our datacenter
// as it was created in a different cluster which will be managing that config entry.
requiresMigration := false
if entry.GetMeta()[common.DatacenterKey] != r.DatacenterName {
if sourceDatacenter != r.DatacenterName {

// Note that there is a special case where we will migrate a config entry
// that wasn't created by the controller if it has the migrate-entry annotation set to true.
// This functionality exists to help folks who are upgrading from older helm
// chart versions where they had previously created config entries themselves but
// now want to manage them through custom resources.
if configEntry.GetObjectMeta().Annotations[common.MigrateEntryKey] != common.MigrateEntryTrue {
return r.syncFailed(ctx, logger, crdCtrl, configEntry, ExternallyManagedConfigError, fmt.Errorf("config entry managed in different datacenter: %q", entry.GetMeta()[common.DatacenterKey]))
return r.syncFailed(ctx, logger, crdCtrl, configEntry, ExternallyManagedConfigError,
sourceDatacenterMismatchErr(sourceDatacenter))
}

requiresMigration = true
Expand Down Expand Up @@ -336,3 +339,16 @@ func containsString(slice []string, s string) bool {
}
return false
}

// sourceDatacenterMismatchErr returns an error for when the source datacenter
// meta key does not match our datacenter. This could be because the config
// entry was created directly in Consul or because it was created by another
// controller in another Consul datacenter.
func sourceDatacenterMismatchErr(sourceDatacenter string) error {
// If the datacenter is empty, then they likely created it in Consul
// directly (vs. another controller in another DC creating it).
if sourceDatacenter == "" {
return fmt.Errorf("config entry already exists in Consul")
}
return fmt.Errorf("config entry managed in different datacenter: %q", sourceDatacenter)
}
Loading

0 comments on commit 3b85413

Please sign in to comment.