Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions e2e/nomostest/config_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,11 @@ var (
//
// All paths must be relative to the test file that is running. There is probably
// a more elegant way to do this.
baseDir = filepath.FromSlash("../..")
outputManifestsDir = filepath.Join(baseDir, ".output", "staging", "oss")
configSyncManifest = filepath.Join(outputManifestsDir, "config-sync-manifest.yaml")
multiConfigMaps = filepath.Join(baseDir, "e2e", "raw-nomos", configSyncManifests, multiConfigMapsName)
baseDir = filepath.FromSlash("../..")
outputManifestsDir = filepath.Join(baseDir, ".output", "staging", "oss")
configSyncManifest = filepath.Join(outputManifestsDir, "config-sync-manifest.yaml")
admissionWebhookManifest = filepath.Join(outputManifestsDir, "admission-webhook.yaml")
multiConfigMaps = filepath.Join(baseDir, "e2e", "raw-nomos", configSyncManifests, multiConfigMapsName)
)

var (
Expand Down Expand Up @@ -245,6 +246,34 @@ func InstallConfigSync(nt *NT) error {
return nil
}

// InstallConfigSyncFromManifest installs ConfigSync on the test cluster by directly
// applying the manifest file using kubectl client-side apply
func InstallConfigSyncFromManifest(nt *NT) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will revert some of the modifications made by the usual InstallConfigSync function used by the test scaffolding. Can this be consolidated to use the same function? I expect you could add an option to use Update instead of Patch/Apply

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate on the 'same function'? I assume you were not suggesting merging the two install functions, the KubeClient.Apply does not achieve necessary field management or removes any legacy fields. Is it now kubectl update instead of kubectl apply that you suggest?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suggesting it would use the client-go Update (KubeClient.Update) method instead of the client-go Apply method (KubeClient.Apply)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The go-client update has a few edge cases too.

When working on the configsync manifest, CRD like clusters.clusterregistry.k8s.io disallows update when there's no resourceversion attached. Something like

    cli_test.go:1477: 2025-09-10 05:42:53.626931518 +0000 UTC ERROR: customresourcedefinitions.apiextensions.k8s.io "clusters.clusterregistry.k8s.io" is invalid: metadata.resourceVersion: Invalid value: 0x0: must be specified for an update

The operator also removes the admission webhook, which will need a special handle when restoring the shared test env, a create instead of update. We could also use preventDrift: true when configuring the configmanagement?

Anyways not sure if the effort is worthy for a deprecated use case test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK added the resourceVersion, hopefully it solves the issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you have to set the resourceVersion when using update. Are you missing context on working with k8s client libraries? If so I can take over the PR - although I don't think this deliverable is really needed until we solve the upgrade issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good chance to learn. The latest change seemed to be working.

nt.T.Log("[SETUP] Installing Config Sync directly from manifest file")

nt.T.Logf("Applying Config Sync manifest directly from %s", configSyncManifest)

out, err := nt.Shell.Kubectl("apply", "-f", configSyncManifest)
if err != nil {
return fmt.Errorf("failed to apply Config Sync manifest: %v\n%s", err, out)
}

nt.T.Logf("Applying multi-repo configmaps from %s", multiConfigMaps)
out, err = nt.Shell.Kubectl("apply", "-f", multiConfigMaps)
if err != nil {
return fmt.Errorf("failed to apply multi-repo configmaps: %v\n%s", err, out)
}

// Apply the admission webhook manifest
nt.T.Logf("Applying admission webhook manifest from %s", admissionWebhookManifest)
out, err = nt.Shell.Kubectl("apply", "-f", admissionWebhookManifest)
if err != nil {
return fmt.Errorf("failed to apply admission webhook manifest: %v\n%s", err, out)
}

return nil
}

// uninstallConfigSync uninstalls ConfigSync on the test cluster
func uninstallConfigSync(nt *NT) error {
nt.T.Log("[CLEANUP] Uninstalling Config Sync")
Expand Down
24 changes: 13 additions & 11 deletions e2e/testcases/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1297,13 +1297,14 @@ func TestApiResourceFormatting(t *testing.T) {
}

func TestNomosMigrate(t *testing.T) {
nt := nomostest.New(t, nomostesting.NomosCLI, ntopts.SkipConfigSyncInstall)
nt := nomostest.New(t, nomostesting.NomosCLI)

nt.T.Cleanup(func() {
// Restore state of Config Sync installation after test
if err := nomostest.InstallConfigSync(nt); err != nil {
if err := nomostest.InstallConfigSyncFromManifest(nt); err != nil {
nt.T.Fatal(err)
}
nt.Must(nt.WatchForAllSyncs())
})
nt.T.Cleanup(func() {
cmObj := &unstructured.Unstructured{
Expand Down Expand Up @@ -1451,11 +1452,11 @@ func TestNomosMigrate(t *testing.T) {
configmanagement.RGControllerName, configmanagement.RGControllerNamespace)
})
tg.Go(func() error {
return nt.Watcher.WatchForNotFound(kinds.Deployment(),
return nt.Watcher.WatchForCurrentStatus(kinds.Deployment(),
core.RootReconcilerName(configsync.RootSyncName), configsync.ControllerNamespace)
})
tg.Go(func() error {
return nt.Watcher.WatchForNotFound(kinds.RootSyncV1Beta1(),
return nt.Watcher.WatchForCurrentStatus(kinds.RootSyncV1Beta1(),
configsync.RootSyncName, configsync.ControllerNamespace)
})
if err := tg.Wait(); err != nil {
Expand All @@ -1464,14 +1465,14 @@ func TestNomosMigrate(t *testing.T) {
}

func TestNomosMigrateMonoRepo(t *testing.T) {
nt := nomostest.New(t, nomostesting.NomosCLI, ntopts.SkipConfigSyncInstall)
nt := nomostest.New(t, nomostesting.NomosCLI)

nt.T.Cleanup(func() {
// Restore state of Config Sync installation after test.
// This also emulates upgrading to the current version after migrating
if err := nomostest.InstallConfigSync(nt); err != nil {
if err := nomostest.InstallConfigSyncFromManifest(nt); err != nil {
nt.T.Fatal(err)
}
nt.Must(nt.WatchForAllSyncs())
})
nt.T.Cleanup(func() {
crds := []string{
Expand Down Expand Up @@ -1707,13 +1708,14 @@ func TestNomosMigrateMonoRepo(t *testing.T) {
// This test case validates the behavior of the uninstall script defined
// at installation/uninstall_configmanagement.sh
func TestACMUninstallScript(t *testing.T) {
nt := nomostest.New(t, nomostesting.NomosCLI, ntopts.SkipConfigSyncInstall)
nt := nomostest.New(t, nomostesting.NomosCLI)

nt.T.Cleanup(func() {
// Restore state of Config Sync installation after test
if err := nomostest.InstallConfigSync(nt); err != nil {
if err := nomostest.InstallConfigSyncFromManifest(nt); err != nil {
nt.T.Fatal(err)
}
nt.Must(nt.WatchForAllSyncs())
})
nt.T.Cleanup(func() {
cmObj := &unstructured.Unstructured{
Expand Down Expand Up @@ -1861,11 +1863,11 @@ func TestACMUninstallScript(t *testing.T) {
configmanagement.RGControllerName, configmanagement.RGControllerNamespace)
})
tg.Go(func() error {
return nt.Watcher.WatchForNotFound(kinds.Deployment(),
return nt.Watcher.WatchForCurrentStatus(kinds.Deployment(),
core.RootReconcilerName(configsync.RootSyncName), configsync.ControllerNamespace)
})
tg.Go(func() error {
return nt.Watcher.WatchForNotFound(kinds.RootSyncV1Beta1(),
return nt.Watcher.WatchForCurrentStatus(kinds.RootSyncV1Beta1(),
configsync.RootSyncName, configsync.ControllerNamespace)
})
if err := tg.Wait(); err != nil {
Expand Down