diff --git a/api/v1alpha1/proxydefaults_types.go b/api/v1alpha1/proxydefaults_types.go index c720d61ff2..bdb7465978 100644 --- a/api/v1alpha1/proxydefaults_types.go +++ b/api/v1alpha1/proxydefaults_types.go @@ -25,6 +25,7 @@ const ( // ProxyDefaults is the Schema for the proxydefaults API // +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type==\"Synced\")].status",description="The sync status of the resource with Consul" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="The age of the resource" type ProxyDefaults struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/servicedefaults_types.go b/api/v1alpha1/servicedefaults_types.go index 5a9d7a8705..67a62c2ddb 100644 --- a/api/v1alpha1/servicedefaults_types.go +++ b/api/v1alpha1/servicedefaults_types.go @@ -20,6 +20,7 @@ const ( // ServiceDefaults is the Schema for the servicedefaults API // +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type==\"Synced\")].status",description="The sync status of the resource with Consul" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="The age of the resource" type ServiceDefaults struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/serviceintentions_types.go b/api/v1alpha1/serviceintentions_types.go index 012bb6b930..b4e4ba938e 100644 --- a/api/v1alpha1/serviceintentions_types.go +++ b/api/v1alpha1/serviceintentions_types.go @@ -72,6 +72,7 @@ type IntentionAction string // ServiceIntentions is the Schema for the serviceintentions API // +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type==\"Synced\")].status",description="The sync status of the resource with Consul" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="The age of the resource" type ServiceIntentions struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/serviceresolver_types.go b/api/v1alpha1/serviceresolver_types.go index 786f510e70..88a4c604fe 100644 --- a/api/v1alpha1/serviceresolver_types.go +++ b/api/v1alpha1/serviceresolver_types.go @@ -22,6 +22,7 @@ const ServiceResolverKubeKind string = "serviceresolver" // ServiceResolver is the Schema for the serviceresolvers API // +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type==\"Synced\")].status",description="The sync status of the resource with Consul" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="The age of the resource" type ServiceResolver struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -247,7 +248,7 @@ type ServiceResolverFailover struct { ServiceSubset string `json:"serviceSubset,omitempty"` // Namespace is the namespace to resolve the requested service from to form // the failover group of instances. If empty the current namespace is used. - Namespace string `json:"namespaces,omitempty"` + Namespace string `json:"namespace,omitempty"` // Datacenters is a fixed list of datacenters to try during failover. Datacenters []string `json:"datacenters,omitempty"` } diff --git a/api/v1alpha1/servicerouter_types.go b/api/v1alpha1/servicerouter_types.go index fd96882271..2b31f55267 100644 --- a/api/v1alpha1/servicerouter_types.go +++ b/api/v1alpha1/servicerouter_types.go @@ -271,6 +271,7 @@ func (in *ServiceRouteDestination) toConsul() *capi.ServiceRouteDestination { // ServiceRouter is the Schema for the servicerouters API // +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type==\"Synced\")].status",description="The sync status of the resource with Consul" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="The age of the resource" type ServiceRouter struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/servicesplitter_types.go b/api/v1alpha1/servicesplitter_types.go index 148210e5cc..b4fa643cca 100644 --- a/api/v1alpha1/servicesplitter_types.go +++ b/api/v1alpha1/servicesplitter_types.go @@ -20,6 +20,7 @@ import ( // ServiceSplitter is the Schema for the servicesplitters API // +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type==\"Synced\")].status",description="The sync status of the resource with Consul" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="The age of the resource" type ServiceSplitter struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml b/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml index 91c950dce0..5fff95ca1e 100644 --- a/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml +++ b/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml @@ -13,6 +13,10 @@ spec: description: The sync status of the resource with Consul name: Synced type: string + - JSONPath: .metadata.creationTimestamp + description: The age of the resource + name: Age + type: date group: consul.hashicorp.com names: kind: ProxyDefaults diff --git a/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml b/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml index 0943a347bb..3ec13128d4 100644 --- a/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml +++ b/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml @@ -13,6 +13,10 @@ spec: description: The sync status of the resource with Consul name: Synced type: string + - JSONPath: .metadata.creationTimestamp + description: The age of the resource + name: Age + type: date group: consul.hashicorp.com names: kind: ServiceDefaults diff --git a/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml b/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml index 6b1a7e683b..db544e096c 100644 --- a/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml +++ b/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml @@ -13,6 +13,10 @@ spec: description: The sync status of the resource with Consul name: Synced type: string + - JSONPath: .metadata.creationTimestamp + description: The age of the resource + name: Age + type: date group: consul.hashicorp.com names: kind: ServiceIntentions diff --git a/config/crd/bases/consul.hashicorp.com_serviceresolvers.yaml b/config/crd/bases/consul.hashicorp.com_serviceresolvers.yaml index 23f1d7abdb..dd202e4897 100644 --- a/config/crd/bases/consul.hashicorp.com_serviceresolvers.yaml +++ b/config/crd/bases/consul.hashicorp.com_serviceresolvers.yaml @@ -13,6 +13,10 @@ spec: description: The sync status of the resource with Consul name: Synced type: string + - JSONPath: .metadata.creationTimestamp + description: The age of the resource + name: Age + type: date group: consul.hashicorp.com names: kind: ServiceResolver @@ -52,7 +56,7 @@ spec: items: type: string type: array - namespaces: + namespace: description: Namespace is the namespace to resolve the requested service from to form the failover group of instances. If empty the current namespace is used. type: string service: diff --git a/config/crd/bases/consul.hashicorp.com_servicerouters.yaml b/config/crd/bases/consul.hashicorp.com_servicerouters.yaml index d7c5bc6779..538558cebf 100644 --- a/config/crd/bases/consul.hashicorp.com_servicerouters.yaml +++ b/config/crd/bases/consul.hashicorp.com_servicerouters.yaml @@ -13,6 +13,10 @@ spec: description: The sync status of the resource with Consul name: Synced type: string + - JSONPath: .metadata.creationTimestamp + description: The age of the resource + name: Age + type: date group: consul.hashicorp.com names: kind: ServiceRouter diff --git a/config/crd/bases/consul.hashicorp.com_servicesplitters.yaml b/config/crd/bases/consul.hashicorp.com_servicesplitters.yaml index 253abba163..863dcdcbc8 100644 --- a/config/crd/bases/consul.hashicorp.com_servicesplitters.yaml +++ b/config/crd/bases/consul.hashicorp.com_servicesplitters.yaml @@ -13,6 +13,10 @@ spec: description: The sync status of the resource with Consul name: Synced type: string + - JSONPath: .metadata.creationTimestamp + description: The age of the resource + name: Age + type: date group: consul.hashicorp.com names: kind: ServiceSplitter diff --git a/controller/configentry_controller.go b/controller/configentry_controller.go index fed1eda278..d45e4dd70e 100644 --- a/controller/configentry_controller.go +++ b/controller/configentry_controller.go @@ -126,7 +126,8 @@ func (r *ConfigEntryController) ReconcileEntry( Namespace: r.consulNamespace(configEntry.ConsulMirroringNS(), configEntry.ConsulGlobalResource()), }) if err != nil { - return ctrl.Result{}, fmt.Errorf("deleting config entry from consul: %w", err) + return r.syncFailed(ctx, logger, crdCtrl, configEntry, ConsulAgentError, + fmt.Errorf("deleting config entry from consul: %w", err)) } logger.Info("deletion from Consul successful") } else { diff --git a/controller/configentry_controller_test.go b/controller/configentry_controller_test.go index d350897c9e..b4e7162d8b 100644 --- a/controller/configentry_controller_test.go +++ b/controller/configentry_controller_test.go @@ -8,6 +8,8 @@ import ( "github.com/go-logr/logr" logrtest "github.com/go-logr/logr/testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/consul-k8s/api/common" "github.com/hashicorp/consul-k8s/api/v1alpha1" capi "github.com/hashicorp/consul/api" @@ -2073,3 +2075,112 @@ func TestConfigEntryControllers_doesNotDeleteUnownedConfig(t *testing.T) { }) } } + +func TestConfigEntryControllers_updatesStatusWhenDeleteFails(t *testing.T) { + ctx := context.Background() + kubeNS := "default" + + s := runtime.NewScheme() + s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.ServiceDefaults{}, &v1alpha1.ServiceSplitter{}) + + defaults := &v1alpha1.ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service", + Namespace: "default", + }, + Spec: v1alpha1.ServiceDefaultsSpec{ + Protocol: "http", + }, + } + + splitter := &v1alpha1.ServiceSplitter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service", + Namespace: "default", + }, + Spec: v1alpha1.ServiceSplitterSpec{ + Splits: v1alpha1.ServiceSplits{ + { + Weight: 100, + Service: "service", + }, + }, + }, + } + + client := fake.NewFakeClientWithScheme(s, defaults, splitter) + + consul, err := testutil.NewTestServerConfigT(t, nil) + require.NoError(t, err) + defer consul.Stop() + + consul.WaitForServiceIntentions(t) + consulClient, err := capi.NewClient(&capi.Config{ + Address: consul.HTTPAddr, + }) + require.NoError(t, err) + + logger := logrtest.TestLogger{T: t} + + svcDefaultsReconciler := ServiceDefaultsController{ + Client: client, + Log: logger, + ConfigEntryController: &ConfigEntryController{ + ConsulClient: consulClient, + DatacenterName: datacenterName, + }, + } + svcSplitterReconciler := ServiceSplitterController{ + Client: client, + Log: logger, + ConfigEntryController: &ConfigEntryController{ + ConsulClient: consulClient, + DatacenterName: datacenterName, + }, + } + + defaultsNamespacedName := types.NamespacedName{ + Namespace: kubeNS, + Name: defaults.Name, + } + + splitterNamespacedName := types.NamespacedName{ + Namespace: kubeNS, + Name: splitter.Name, + } + + // Create config entries for service-defaults and service-splitter. + resp, err := svcDefaultsReconciler.Reconcile(ctrl.Request{NamespacedName: defaultsNamespacedName}) + require.NoError(t, err) + require.False(t, resp.Requeue) + + resp, err = svcSplitterReconciler.Reconcile(ctrl.Request{NamespacedName: splitterNamespacedName}) + require.NoError(t, err) + require.False(t, resp.Requeue) + + err = client.Get(ctx, defaultsNamespacedName, defaults) + require.NoError(t, err) + + // Update service-defaults with deletion timestamp so that it attempts deletion on reconcile. + defaults.ObjectMeta.DeletionTimestamp = &metav1.Time{Time: time.Now()} + err = client.Update(ctx, defaults) + require.NoError(t, err) + + // Reconcile should fail as the service-splitter still required the service-defaults causing the delete operation on Consul to fail. + resp, err = svcDefaultsReconciler.Reconcile(ctrl.Request{NamespacedName: defaultsNamespacedName}) + require.EqualError(t, err, "deleting config entry from consul: Unexpected response code: 500 (discovery chain \"service\" uses a protocol \"tcp\" that does not permit advanced routing or splitting behavior)") + require.False(t, resp.Requeue) + + err = client.Get(ctx, defaultsNamespacedName, defaults) + require.NoError(t, err) + + // Ensure the status of the resource is updated to display failure reason. + syncCondition := defaults.GetCondition(v1alpha1.ConditionSynced) + expectedCondition := &v1alpha1.Condition{ + Type: v1alpha1.ConditionSynced, + Status: corev1.ConditionFalse, + Reason: ConsulAgentError, + Message: "deleting config entry from consul: Unexpected response code: 500 (discovery chain \"service\" uses a protocol \"tcp\" that does not permit advanced routing or splitting behavior)", + } + require.True(t, cmp.Equal(syncCondition, expectedCondition, cmpopts.IgnoreFields(v1alpha1.Condition{}, "LastTransitionTime"))) +} diff --git a/go.sum b/go.sum index 8946608ad5..55cdff5c07 100644 --- a/go.sum +++ b/go.sum @@ -252,8 +252,6 @@ github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/hashicorp/consul/api v1.4.1-0.20201007080954-aa0f5ff839c5 h1:mHgaDaPdf0z3UG3G2UpCINFMRKhzi1DLNoOp6sIQWnU= github.com/hashicorp/consul/api v1.4.1-0.20201007080954-aa0f5ff839c5/go.mod h1:1NSuaUUkFaJzMasbfq/11wKYWSR67Xn6r2DXKhuDNFg= -github.com/hashicorp/consul/api v1.6.0 h1:SZB2hQW8AcTOpfDmiVblQbijxzsRuiyy0JpHfabvHio= -github.com/hashicorp/consul/api v1.6.0/go.mod h1:1NSuaUUkFaJzMasbfq/11wKYWSR67Xn6r2DXKhuDNFg= github.com/hashicorp/consul/sdk v0.4.1-0.20201006182405-a2a8e9c7839a h1:yclqizoDCodLeiAUg1Siaodz3hvIBxzH8A2GnjY74EU= github.com/hashicorp/consul/sdk v0.4.1-0.20201006182405-a2a8e9c7839a/go.mod h1:fY08Y9z5SvJqevyZNy6WWPXiG3KwBPAvlcdx16zZ0fM= github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=