From 8064b0659e7707b5ec618740b6342d420503165d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan-Luis=20de=20Sousa-Valadas=20Casta=C3=B1o?= Date: Fri, 24 Jan 2025 15:59:18 +0100 Subject: [PATCH] Removed APIs, ensure object is available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior implementation checked that there were no objects present in the removed APIs, however, it failed if the object was available on both the removed API and the present API. This behavior is incorrect and now for each object present in the removed API, we verify it's present in the new version (if available). Signed-off-by: Juan-Luis de Sousa-Valadas Castaño --- pkg/autopilot/checks/checks.go | 33 ++++++++++++++++++++++-- pkg/autopilot/checks/removedapis.go | 22 +++++++++------- pkg/autopilot/checks/removedapis_test.go | 26 +++++++++++++------ 3 files changed, 62 insertions(+), 19 deletions(-) diff --git a/pkg/autopilot/checks/checks.go b/pkg/autopilot/checks/checks.go index 3cd8395d3392..e6604f652612 100644 --- a/pkg/autopilot/checks/checks.go +++ b/pkg/autopilot/checks/checks.go @@ -17,9 +17,11 @@ package checks import ( "context" "fmt" + "strings" "github.com/k0sproject/k0s/pkg/kubernetes" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/metadata" @@ -51,6 +53,12 @@ func CanUpdate(ctx context.Context, log logrus.FieldLogger, clientFactory kubern } for _, ar := range r.APIResources { + // Skip resources which don't have the same name and kind. This is to skip + // subresources such as FlowSchema/Status + if strings.Contains(ar.Name, "/") { + continue + } + gv := gv // Copy over the default GroupVersion from the list // Apply resource-specific overrides if ar.Group != "" { @@ -60,7 +68,7 @@ func CanUpdate(ctx context.Context, log logrus.FieldLogger, clientFactory kubern gv.Version = ar.Version } - removedInVersion := removedInVersion(gv.WithKind(ar.Kind)) + removedInVersion, currentVersion := removedInVersion(gv.WithKind(ar.Kind)) if removedInVersion == "" || semver.Compare(newVersion, removedInVersion) < 0 { continue } @@ -84,7 +92,28 @@ func CanUpdate(ctx context.Context, log logrus.FieldLogger, clientFactory kubern } if found := len(metas.Items); found > 0 { - return fmt.Errorf("%s.%s %s has been removed in Kubernetes %s, but there are %d such resources in the cluster", ar.Name, gv.Group, gv.Version, removedInVersion, found) + if currentVersion == "" { + return fmt.Errorf("%s.%s %s has been removed in Kubernetes %s, but there are %d such resources in the cluster", ar.Name, gv.Group, gv.Version, removedInVersion, found) + } + // If we find removed APIs, it could be because the APIserver is serving the same object with an older GVK + // for compatibility reasons while the current good API still works. + newGV := gv + newGV.Version = currentVersion + outdatedItems := []metav1.PartialObjectMetadata{} + for _, item := range metas.Items { + // Currently none of the deleted resources are namespaced, so we can skip the namespace check. + // However we keep it in the list so that it breaks if we add a namespaced resource. + _, err := metaClient.Resource(newGV.WithResource(ar.Name)). + Get(ctx, item.GetName(), metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + outdatedItems = append(outdatedItems, item) + } else if err != nil { + return err + } + } + if foundOutdated := len(outdatedItems); foundOutdated > 0 { + return fmt.Errorf("%s.%s %s has been removed in Kubernetes %s, but there are %d such resources in the cluster", ar.Name, gv.Group, gv.Version, removedInVersion, found) + } } } } diff --git a/pkg/autopilot/checks/removedapis.go b/pkg/autopilot/checks/removedapis.go index 56105b666c3d..54b66503c3ed 100644 --- a/pkg/autopilot/checks/removedapis.go +++ b/pkg/autopilot/checks/removedapis.go @@ -23,10 +23,14 @@ import ( type removedAPI struct { group, version, kind, removedInVersion string + // currentVersion declares a version that is still supported for the Group Kind. + // If it's empty, it means that the Group Kind is removed in the removedInVersion. + currentVersion string } -// Returns the Kubernetes version in which candidate has been removed, if any. -func removedInVersion(candidate schema.GroupVersionKind) string { +// If candidate has been removed, returns the kubernetes version in which it was removed +// and the current version for Group Kind. +func removedInVersion(candidate schema.GroupVersionKind) (string, string) { if idx, found := sort.Find(len(removedGVKs), func(i int) int { if cmp := cmp.Compare(candidate.Group, removedGVKs[i].group); cmp != 0 { return cmp @@ -36,17 +40,17 @@ func removedInVersion(candidate schema.GroupVersionKind) string { } return cmp.Compare(candidate.Kind, removedGVKs[i].kind) }); found { - return removedGVKs[idx].removedInVersion + return removedGVKs[idx].removedInVersion, removedGVKs[idx].currentVersion } - return "" + return "", "" } // Sorted array of removed APIs. var removedGVKs = [...]removedAPI{ - {"flowcontrol.apiserver.k8s.io", "v1beta2", "FlowSchema", "v1.29.0"}, - {"flowcontrol.apiserver.k8s.io", "v1beta2", "PriorityLevelConfiguration", "v1.29.0"}, - {"flowcontrol.apiserver.k8s.io", "v1beta3", "FlowSchema", "v1.32.0"}, - {"flowcontrol.apiserver.k8s.io", "v1beta3", "PriorityLevelConfiguration", "v1.32.0"}, - {"k0s.k0sproject.example.com", "v1beta1", "RemovedCRD", "v99.99.99"}, // This is a test entry + {"flowcontrol.apiserver.k8s.io", "v1beta2", "FlowSchema", "v1.29.0", "v1beta3"}, + {"flowcontrol.apiserver.k8s.io", "v1beta2", "PriorityLevelConfiguration", "v1.29.0", "v1"}, + {"flowcontrol.apiserver.k8s.io", "v1beta3", "FlowSchema", "v1.32.0", "v1"}, + {"flowcontrol.apiserver.k8s.io", "v1beta3", "PriorityLevelConfiguration", "v1.32.0", "v1"}, + {"k0s.k0sproject.example.com", "v1beta1", "RemovedCRD", "v99.99.99", ""}, // This is a test entry } diff --git a/pkg/autopilot/checks/removedapis_test.go b/pkg/autopilot/checks/removedapis_test.go index 46bacf83b569..e9bd4640f3d5 100644 --- a/pkg/autopilot/checks/removedapis_test.go +++ b/pkg/autopilot/checks/removedapis_test.go @@ -19,9 +19,8 @@ import ( "slices" "testing" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime/schema" ) func TestRemovedGVKs(t *testing.T) { @@ -36,10 +35,21 @@ func TestRemovedGVKs(t *testing.T) { }), "removedGVKs needs to be sorted, so that it can be used for binary searches") // Test two random entries at the top and the bottom of the list - assert.Equal(t, "v1.22.0", removedInVersion(schema.GroupVersionKind{ - Group: "apiregistration.k8s.io", Version: "v1beta1", Kind: "APIService", - })) - assert.Equal(t, "v1.27.0", removedInVersion(schema.GroupVersionKind{ - Group: "storage.k8s.io", Version: "v1beta1", Kind: "CSIStorageCapacity", - })) + version, currentVersion := removedInVersion(schema.GroupVersionKind{ + Group: "flowcontrol.apiserver.k8s.io", Version: "v1beta2", Kind: "FlowSchema", + }) + assert.Equal(t, "v1.29.0", version) + assert.Equal(t, "v1beta3", currentVersion) + + version, currentVersion = removedInVersion(schema.GroupVersionKind{ + Group: "k0s.k0sproject.example.com", Version: "v1beta1", Kind: "RemovedCRD", + }) + assert.Equal(t, "v99.99.99", version) + assert.Equal(t, "", currentVersion) + + version, currentVersion = removedInVersion(schema.GroupVersionKind{ + Group: "k0s.k0sproject.example.com", Version: "v1beta1", Kind: "MustFail", + }) + assert.Equal(t, "", version) + assert.Equal(t, "", currentVersion) }