Merge pull request kubernetes#124877 from nilekhc/svm-crd-flake

fix: fixes update CRD check

Author: k8s-ci-robot

test/integration/storageversionmigrator/storageversionmigrator_test.go

@@ -181,7 +181,7 @@ func TestStorageVersionMigrationWithCRD(t *testing.T) {
 	shutdownServer := svmTest.createConversionWebhook(ctx, t, certCtx)
 
 	// add v2 for serving only
-	svmTest.updateCRD(ctx, t, crd.Name, v2CRDVersion)
+	svmTest.updateCRD(ctx, t, crd.Name, v2CRDVersion, []string{"v1", "v2"}, "v1")
 
 	// create another CR
 	cr2 := svmTest.createCR(ctx, t, "cr2", "v2")
@@ -195,7 +195,7 @@ func TestStorageVersionMigrationWithCRD(t *testing.T) {
 	}
 
 	// add v2 as storage version
-	svmTest.updateCRD(ctx, t, crd.Name, v2StorageCRDVersion)
+	svmTest.updateCRD(ctx, t, crd.Name, v2StorageCRDVersion, []string{"v1", "v2"}, "v2")
 
 	// create CR with v1
 	cr3 := svmTest.createCR(ctx, t, "cr3", "v1")
@@ -257,7 +257,7 @@ func TestStorageVersionMigrationWithCRD(t *testing.T) {
 	}
 
 	// update CRD to v1 not serving and storage followed by webhook shutdown
-	svmTest.updateCRD(ctx, t, crd.Name, v1NotServingCRDVersion)
+	svmTest.updateCRD(ctx, t, crd.Name, v1NotServingCRDVersion, []string{"v2"}, "v2")
 	shutdownServer()
 
 	// assert RV and Generations of CRs

test/integration/storageversionmigrator/util.go

@@ -26,7 +26,9 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"testing"
@@ -46,6 +48,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	auditinternal "k8s.io/apiserver/pkg/apis/audit"
 	auditv1 "k8s.io/apiserver/pkg/apis/audit/v1"
+	endpointsdiscovery "k8s.io/apiserver/pkg/endpoints/discovery"
 	"k8s.io/apiserver/pkg/storage/storagebackend"
 	"k8s.io/client-go/discovery"
 	cacheddiscovery "k8s.io/client-go/discovery/cached/memory"
@@ -245,6 +248,7 @@ type svmTest struct {
 	client                      clientset.Interface
 	clientConfig                *rest.Config
 	dynamicClient               *dynamic.DynamicClient
+	discoveryClient             *discovery.DiscoveryClient
 	storageConfig               *storagebackend.Config
 	server                      *kubeapiservertesting.TestServer
 	apiextensionsclient         *apiextensionsclientset.Clientset
@@ -350,6 +354,7 @@ func svmSetup(ctx context.Context, t *testing.T) *svmTest {
 		client:                      clientSet,
 		clientConfig:                server.ClientConfig,
 		dynamicClient:               dynamicClient,
+		discoveryClient:             rvDiscoveryClient,
 		policyFile:                  policyFile,
 		logFile:                     logFile,
 		filePathForEncryptionConfig: filePathForEncryptionConfig,
@@ -484,29 +489,6 @@ func (svm *svmTest) updateFile(t *testing.T, configDir, filename string, newCont
 	}
 }
 
-// func (svm *svmTest) createSVMResource(ctx context.Context, t *testing.T, name string) (
-// 	*svmv1alpha1.StorageVersionMigration,
-// 	error,
-// ) {
-// 	t.Helper()
-// 	svmResource := &svmv1alpha1.StorageVersionMigration{
-// 		ObjectMeta: metav1.ObjectMeta{
-// 			Name: name,
-// 		},
-// 		Spec: svmv1alpha1.StorageVersionMigrationSpec{
-// 			Resource: svmv1alpha1.GroupVersionResource{
-// 				Group:    "",
-// 				Version:  "v1",
-// 				Resource: "secrets",
-// 			},
-// 		},
-// 	}
-//
-// 	return svm.client.StoragemigrationV1alpha1().
-// 		StorageVersionMigrations().
-// 		Create(ctx, svmResource, metav1.CreateOptions{})
-// }
-
 func (svm *svmTest) createSVMResource(ctx context.Context, t *testing.T, name string, gvr svmv1alpha1.GroupVersionResource) (
 	*svmv1alpha1.StorageVersionMigration,
 	error,
@@ -757,27 +739,74 @@ func (svm *svmTest) updateCRD(
 	t *testing.T,
 	crdName string,
 	updatesCRDVersions []apiextensionsv1.CustomResourceDefinitionVersion,
-) *apiextensionsv1.CustomResourceDefinition {
+	expectedServingVersions []string,
+	expectedStorageVersion string,
+) {
 	t.Helper()
 
 	var err error
-	_, err = crdintegration.UpdateV1CustomResourceDefinitionWithRetry(svm.apiextensionsclient, crdName, func(c *apiextensionsv1.CustomResourceDefinition) {
+	crd, err := crdintegration.UpdateV1CustomResourceDefinitionWithRetry(svm.apiextensionsclient, crdName, func(c *apiextensionsv1.CustomResourceDefinition) {
 		c.Spec.Versions = updatesCRDVersions
 	})
 	if err != nil {
 		t.Fatalf("Failed to update CRD: %v", err)
 	}
 
-	crd, err := svm.apiextensionsclient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, crdName, metav1.GetOptions{})
-	if err != nil {
-		t.Fatalf("Failed to get CRD: %v", err)
-	}
+	svm.waitForCRDUpdate(ctx, t, crd.Spec.Names.Kind, expectedServingVersions, expectedStorageVersion)
+}
 
-	// TODO: wrap all actions after updateCRD with wait loops so we do not need this sleep
-	//  it is currently necessary because we update the CRD but do not otherwise guarantee that the updated config is active
-	time.Sleep(10 * time.Second)
+func (svm *svmTest) waitForCRDUpdate(
+	ctx context.Context,
+	t *testing.T,
+	crdKind string,
+	expectedServingVersions []string,
+	expectedStorageVersion string,
+) {
+	t.Helper()
 
-	return crd
+	err := wait.PollUntilContextTimeout(
+		ctx,
+		500*time.Millisecond,
+		wait.ForeverTestTimeout,
+		true,
+		func(ctx context.Context) (bool, error) {
+			apiGroups, _, err := svm.discoveryClient.ServerGroupsAndResources()
+			if err != nil {
+				return false, fmt.Errorf("failed to get server groups and resources: %w", err)
+			}
+			for _, api := range apiGroups {
+				if api.Name == crdGroup {
+					var servingVersions []string
+					for _, apiVersion := range api.Versions {
+						servingVersions = append(servingVersions, apiVersion.Version)
+					}
+					sort.Strings(servingVersions)
+
+					// Check if the serving versions are as expected
+					if reflect.DeepEqual(expectedServingVersions, servingVersions) {
+						expectedHash := endpointsdiscovery.StorageVersionHash(crdGroup, expectedStorageVersion, crdKind)
+						resourceList, err := svm.discoveryClient.ServerResourcesForGroupVersion(crdGroup + "/" + api.PreferredVersion.Version)
+						if err != nil {
+							return false, fmt.Errorf("failed to get server resources for group version: %w", err)
+						}
+
+						// Check if the storage version is as expected
+						for _, resource := range resourceList.APIResources {
+							if resource.Kind == crdKind {
+								if resource.StorageVersionHash == expectedHash {
+									return true, nil
+								}
+							}
+						}
+					}
+				}
+			}
+			return false, nil
+		},
+	)
+	if err != nil {
+		t.Fatalf("Failed to update a CRD: Name: %s, Err: %v", crdName, err)
+	}
 }
 
 func (svm *svmTest) createCR(ctx context.Context, t *testing.T, crName, version string) *unstructured.Unstructured {