diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 86025fc09fc..0d8e4217056 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -4006,6 +4006,8 @@ type NamespaceStatus struct { // Phase is the current lifecycle phase of the namespace. // +optional Phase NamespacePhase + // +optional + Conditions []NamespaceCondition } type NamespacePhase string @@ -4018,6 +4020,30 @@ const ( NamespaceTerminating NamespacePhase = "Terminating" ) +// NamespaceConditionType defines constants reporting on status during namespace lifetime and deletion progress +type NamespaceConditionType string + +// These are valid conditions of a namespace. +const ( + NamespaceDeletionDiscoveryFailure NamespaceConditionType = "NamespaceDeletionDiscoveryFailure" + NamespaceDeletionContentFailure NamespaceConditionType = "NamespaceDeletionContentFailure" + NamespaceDeletionGVParsingFailure NamespaceConditionType = "NamespaceDeletionGroupVersionParsingFailure" +) + +// NamespaceCondition contains details about state of namespace. +type NamespaceCondition struct { + // Type of namespace controller condition. + Type NamespaceConditionType + // Status of the condition, one of True, False, Unknown. + Status ConditionStatus + // +optional + LastTransitionTime metav1.Time + // +optional + Reason string + // +optional + Message string +} + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // A namespace provides a scope for Names. diff --git a/pkg/controller/namespace/deletion/namespaced_resources_deleter.go b/pkg/controller/namespace/deletion/namespaced_resources_deleter.go index 1e702fba6fb..c0d27b7a438 100644 --- a/pkg/controller/namespace/deletion/namespaced_resources_deleter.go +++ b/pkg/controller/namespace/deletion/namespaced_resources_deleter.go @@ -134,7 +134,7 @@ func (d *namespacedResourcesDeleter) Delete(nsName string) error { } // there may still be content for us to remove - estimate, err := d.deleteAllContent(namespace.Name, *namespace.DeletionTimestamp) + estimate, err := d.deleteAllContent(namespace) if err != nil { return err } @@ -292,7 +292,7 @@ func (d *namespacedResourcesDeleter) updateNamespaceStatusFunc(namespace *v1.Nam } newNamespace := v1.Namespace{} newNamespace.ObjectMeta = namespace.ObjectMeta - newNamespace.Status = namespace.Status + newNamespace.Status = *namespace.Status.DeepCopy() newNamespace.Status.Phase = v1.NamespaceTerminating return d.nsClient.UpdateStatus(&newNamespace) } @@ -480,8 +480,11 @@ func (d *namespacedResourcesDeleter) deleteAllContentForGroupVersionResource( // deleteAllContent will use the dynamic client to delete each resource identified in groupVersionResources. // It returns an estimate of the time remaining before the remaining resources are deleted. // If estimate > 0, not all resources are guaranteed to be gone. -func (d *namespacedResourcesDeleter) deleteAllContent(namespace string, namespaceDeletedAt metav1.Time) (int64, error) { +func (d *namespacedResourcesDeleter) deleteAllContent(ns *v1.Namespace) (int64, error) { + namespace := ns.Name + namespaceDeletedAt := *ns.DeletionTimestamp var errs []error + conditionUpdater := namespaceConditionUpdater{} estimate := int64(0) klog.V(4).Infof("namespace controller - deleteAllContent - namespace: %s", namespace) @@ -489,6 +492,7 @@ func (d *namespacedResourcesDeleter) deleteAllContent(namespace string, namespac if err != nil { // discovery errors are not fatal. We often have some set of resources we can operate against even if we don't have a complete list errs = append(errs, err) + conditionUpdater.ProcessDiscoverResourcesErr(err) } // TODO(sttts): get rid of opCache and pass the verbs (especially "deletecollection") down into the deleter deletableResources := discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"delete"}}, resources) @@ -496,6 +500,7 @@ func (d *namespacedResourcesDeleter) deleteAllContent(namespace string, namespac if err != nil { // discovery errors are not fatal. We often have some set of resources we can operate against even if we don't have a complete list errs = append(errs, err) + conditionUpdater.ProcessGroupVersionErr(err) } for gvr := range groupVersionResources { gvrEstimate, err := d.deleteAllContentForGroupVersionResource(gvr, namespace, namespaceDeletedAt) @@ -503,12 +508,19 @@ func (d *namespacedResourcesDeleter) deleteAllContent(namespace string, namespac // If there is an error, hold on to it but proceed with all the remaining // groupVersionResources. errs = append(errs, err) + conditionUpdater.ProcessDeleteContentErr(err) } if gvrEstimate > estimate { estimate = gvrEstimate } } + if len(errs) > 0 { + if hasChanged := conditionUpdater.Update(ns); hasChanged { + if _, err = d.nsClient.UpdateStatus(ns); err != nil { + utilruntime.HandleError(fmt.Errorf("couldn't update status condition for namespace %q: %v", namespace, err)) + } + } return estimate, utilerrors.NewAggregate(errs) } klog.V(4).Infof("namespace controller - deleteAllContent - namespace: %s, estimate: %v", namespace, estimate) diff --git a/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go b/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go index 7b2bf098821..5ae7d34c725 100644 --- a/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go +++ b/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go @@ -137,6 +137,7 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio kubeClientActionSet sets.String metadataClientActionSet sets.String gvrError error + expectErrorOnDelete error }{ "pending-finalize": { testNamespace: testNamespacePendingFinalize, @@ -165,6 +166,17 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio metadataClientActionSet: sets.NewString(), gvrError: fmt.Errorf("test error"), }, + "groupVersionResourceErr-finalize": { + testNamespace: testNamespacePendingFinalize, + kubeClientActionSet: sets.NewString( + strings.Join([]string{"get", "namespaces", ""}, "-"), + strings.Join([]string{"list", "pods", ""}, "-"), + strings.Join([]string{"update", "namespaces", "status"}, "-"), + ), + dynamicClientActionSet: dynamicClientActionSet, + gvrError: fmt.Errorf("test error"), + expectErrorOnDelete: fmt.Errorf("test error"), + }, } for scenario, testInput := range scenarios { @@ -179,11 +191,11 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio } fn := func() ([]*metav1.APIResourceList, error) { - return resources, nil + return resources, testInput.gvrError } d := NewNamespacedResourcesDeleter(mockClient.CoreV1().Namespaces(), metadataClient, mockClient.CoreV1(), fn, v1.FinalizerKubernetes, true) - if err := d.Delete(testInput.testNamespace.Name); err != nil { - t.Errorf("scenario %s - Unexpected error when synching namespace %v", scenario, err) + if err := d.Delete(testInput.testNamespace.Name); !matchErrors(err, testInput.expectErrorOnDelete) { + t.Errorf("scenario %s - expected error %q when syncing namespace, got %q, %v", scenario, testInput.expectErrorOnDelete, err, testInput.expectErrorOnDelete == err) } // validate traffic from kube client @@ -271,6 +283,17 @@ func TestSyncNamespaceThatIsActive(t *testing.T) { } } +// matchError returns true if errors match, false if they don't, compares by error message only for convenience which should be sufficient for these tests +func matchErrors(e1, e2 error) bool { + if e1 == nil && e2 == nil { + return true + } + if e1 != nil && e2 != nil { + return e1.Error() == e2.Error() + } + return false +} + // testServerAndClientConfig returns a server that listens and a config that can reference it func testServerAndClientConfig(handler func(http.ResponseWriter, *http.Request)) (*httptest.Server, *restclient.Config) { srv := httptest.NewServer(http.HandlerFunc(handler)) diff --git a/pkg/controller/namespace/deletion/status_condition_utils.go b/pkg/controller/namespace/deletion/status_condition_utils.go new file mode 100644 index 00000000000..6cd0fc35471 --- /dev/null +++ b/pkg/controller/namespace/deletion/status_condition_utils.go @@ -0,0 +1,171 @@ +/* +Copyright 2019 The Kubernetes Authors. + +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 +limitations under the License. +*/ + +package deletion + +import ( + "fmt" + "sort" + "strings" + + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/discovery" +) + +// NamespaceConditionUpdater interface that translates namespace deleter errors +// into namespace status conditions. +type NamespaceConditionUpdater interface { + ProcessDiscoverResourcesErr(e error) + ProcessGroupVersionErr(e error) + ProcessDeleteContentErr(e error) + Update(*v1.Namespace) bool +} + +type namespaceConditionUpdater struct { + newConditions []v1.NamespaceCondition + deleteContentErrors []error +} + +var _ NamespaceConditionUpdater = &namespaceConditionUpdater{} + +var ( + // conditionTypes Namespace condition types that are maintained by namespace_deleter controller. + conditionTypes = []v1.NamespaceConditionType{ + v1.NamespaceDeletionDiscoveryFailure, + v1.NamespaceDeletionGVParsingFailure, + v1.NamespaceDeletionContentFailure, + } + okMessages = map[v1.NamespaceConditionType]string{ + v1.NamespaceDeletionDiscoveryFailure: "All resources successfully discovered", + v1.NamespaceDeletionGVParsingFailure: "All legacy kube types successfully parsed", + v1.NamespaceDeletionContentFailure: "All content successfully deleted", + } + okReasons = map[v1.NamespaceConditionType]string{ + v1.NamespaceDeletionDiscoveryFailure: "ResourcesDiscovered", + v1.NamespaceDeletionGVParsingFailure: "ParsedGroupVersions", + v1.NamespaceDeletionContentFailure: "ContentDeleted", + } +) + +// ProcessGroupVersionErr creates error condition if parsing GroupVersion of resources fails. +func (u *namespaceConditionUpdater) ProcessGroupVersionErr(err error) { + d := v1.NamespaceCondition{ + Type: v1.NamespaceDeletionGVParsingFailure, + Status: v1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "GroupVersionParsingFailed", + Message: err.Error(), + } + u.newConditions = append(u.newConditions, d) +} + +// ProcessDiscoverResourcesErr creates error condition from ErrGroupDiscoveryFailed. +func (u *namespaceConditionUpdater) ProcessDiscoverResourcesErr(err error) { + var msg string + if derr, ok := err.(*discovery.ErrGroupDiscoveryFailed); ok { + msg = fmt.Sprintf("Discovery failed for some groups, %d failing: %v", len(derr.Groups), err) + } else { + msg = err.Error() + } + d := v1.NamespaceCondition{ + Type: v1.NamespaceDeletionDiscoveryFailure, + Status: v1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "DiscoveryFailed", + Message: msg, + } + u.newConditions = append(u.newConditions, d) + +} + +// ProcessDeleteContentErr creates error condition from multiple delete content errors. +func (u *namespaceConditionUpdater) ProcessDeleteContentErr(err error) { + u.deleteContentErrors = append(u.deleteContentErrors, err) +} + +// Update compiles processed errors from namespace deletion into status conditions. +func (u *namespaceConditionUpdater) Update(ns *v1.Namespace) bool { + if c := getCondition(u.newConditions, v1.NamespaceDeletionContentFailure); c == nil { + if c := makeDeleteContentCondition(u.deleteContentErrors); c != nil { + u.newConditions = append(u.newConditions, *c) + } + } + return updateConditions(&ns.Status, u.newConditions) +} + +func makeDeleteContentCondition(err []error) *v1.NamespaceCondition { + if len(err) == 0 { + return nil + } + msgs := make([]string, 0, len(err)) + for _, e := range err { + msgs = append(msgs, e.Error()) + } + sort.Strings(msgs) + return &v1.NamespaceCondition{ + Type: v1.NamespaceDeletionContentFailure, + Status: v1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "ContentDeletionFailed", + Message: fmt.Sprintf("Failed to delete all resource types, %d remaining: %v", len(err), strings.Join(msgs, ", ")), + } +} + +func updateConditions(status *v1.NamespaceStatus, newConditions []v1.NamespaceCondition) (hasChanged bool) { + for _, conditionType := range conditionTypes { + newCondition := getCondition(newConditions, conditionType) + oldCondition := getCondition(status.Conditions, conditionType) + if newCondition == nil && oldCondition == nil { + // both are nil, no update necessary + continue + } + if oldCondition == nil { + // only new condition of this type exists, add to the list + status.Conditions = append(status.Conditions, *newCondition) + hasChanged = true + } else if newCondition == nil { + // only old condition of this type exists, set status to false + if oldCondition.Status != v1.ConditionFalse { + oldCondition.Status = v1.ConditionFalse + oldCondition.Message = okMessages[conditionType] + oldCondition.Reason = okReasons[conditionType] + oldCondition.LastTransitionTime = metav1.Now() + hasChanged = true + } + } else if oldCondition.Message != newCondition.Message { + // old condition needs to be updated + if oldCondition.Status != newCondition.Status { + oldCondition.LastTransitionTime = metav1.Now() + } + oldCondition.Type = newCondition.Type + oldCondition.Status = newCondition.Status + oldCondition.Reason = newCondition.Reason + oldCondition.Message = newCondition.Message + hasChanged = true + } + } + return +} + +func getCondition(conditions []v1.NamespaceCondition, conditionType v1.NamespaceConditionType) *v1.NamespaceCondition { + for i := range conditions { + if conditions[i].Type == conditionType { + return &(conditions[i]) + } + } + return nil +} diff --git a/pkg/controller/namespace/namespace_controller.go b/pkg/controller/namespace/namespace_controller.go index 73a6912ea55..ef6720b0034 100644 --- a/pkg/controller/namespace/namespace_controller.go +++ b/pkg/controller/namespace/namespace_controller.go @@ -145,7 +145,7 @@ func (nm *NamespaceController) worker() { } else { // rather than wait for a full resync, re-add the namespace to the queue to be processed nm.queue.AddRateLimited(key) - utilruntime.HandleError(err) + utilruntime.HandleError(fmt.Errorf("deletion of namespace %v failed: %v", key, err)) } return false } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 9bf48ecf15c..a8ecba55c65 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4617,6 +4617,12 @@ type NamespaceStatus struct { // More info: https://kubernetes.io/docs/tasks/administer-cluster/namespaces/ // +optional Phase NamespacePhase `json:"phase,omitempty" protobuf:"bytes,1,opt,name=phase,casttype=NamespacePhase"` + + // Represents the latest available observations of a namespace's current state. + // +optional + // +patchMergeKey=type + // +patchStrategy=merge + Conditions []NamespaceCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,2,rep,name=conditions"` } type NamespacePhase string @@ -4629,6 +4635,32 @@ const ( NamespaceTerminating NamespacePhase = "Terminating" ) +type NamespaceConditionType string + +// These are valid conditions of a namespace. +const ( + // NamespaceDeletionDiscoveryFailure contains information about namespace deleter errors during resource discovery. + NamespaceDeletionDiscoveryFailure NamespaceConditionType = "NamespaceDeletionDiscoveryFailure" + // NamespaceDeletionContentFailure contains information about namespace deleter errors during deletion of resources. + NamespaceDeletionContentFailure NamespaceConditionType = "NamespaceDeletionContentFailure" + // NamespaceDeletionGVParsingFailure contains information about namespace deleter errors parsing GV for legacy types. + NamespaceDeletionGVParsingFailure NamespaceConditionType = "NamespaceDeletionGroupVersionParsingFailure" +) + +// NamespaceCondition contains details about state of namespace. +type NamespaceCondition struct { + // Type of namespace controller condition. + Type NamespaceConditionType `json:"type" protobuf:"bytes,1,opt,name=type,casttype=NamespaceConditionType"` + // Status of the condition, one of True, False, Unknown. + Status ConditionStatus `json:"status" protobuf:"bytes,2,opt,name=status,casttype=ConditionStatus"` + // +optional + LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty" protobuf:"bytes,4,opt,name=lastTransitionTime"` + // +optional + Reason string `json:"reason,omitempty" protobuf:"bytes,5,opt,name=reason"` + // +optional + Message string `json:"message,omitempty" protobuf:"bytes,6,opt,name=message"` +} + // +genclient // +genclient:nonNamespaced // +genclient:skipVerbs=deleteCollection