From c5aad52acd6958d7747dea0c203589442bf21162 Mon Sep 17 00:00:00 2001 From: Mukul Anand Date: Thu, 4 Sep 2025 20:04:45 +0000 Subject: [PATCH 1/4] backport of commit 52244356c620ac48852bfbb5ff8004086a7c9fbc --- .../subcommand/install-cni/command.go | 126 +++++++++++++----- 1 file changed, 92 insertions(+), 34 deletions(-) diff --git a/control-plane/subcommand/install-cni/command.go b/control-plane/subcommand/install-cni/command.go index 4c4ef180ed..9bb9005248 100644 --- a/control-plane/subcommand/install-cni/command.go +++ b/control-plane/subcommand/install-cni/command.go @@ -193,6 +193,9 @@ func (c *Command) Run(args []string) int { // annotation during connect inject so that multus knows to run the consul-cni plugin. c.logger.Info("Multus enabled, using multus NetworkAttachementDefinition for configuration") } + + errCh := make(chan error, 2) + var wg sync.WaitGroup // watch for changes in the default cni serviceaccount token directory if cfg.AutorotateToken { // if autorotate-token is enabled, we need to watch the token file for changes and copy it to the host @@ -200,37 +203,68 @@ func (c *Command) Run(args []string) int { sourceTokenPath := cfg.CNITokenPath hostTokenPath := cfg.CNIHostTokenPath go func() { + wg.Add(1) + defer wg.Done() if err := c.tokenFileWatcher(ctx, sourceTokenPath, hostTokenPath); err != nil { c.logger.Error("Token file watcher failed", "error", err) + errCh <- err } }() } // Watch for changes in the cniNetDir directory and fix/install the config file if need be. - err = c.directoryWatcher(ctx, cfg, cfg.CNINetDir, cfgFile) - if err != nil { - c.logger.Error("error with directory watcher", "error", err) - return 1 + go func() { + wg.Add(1) + defer wg.Done() + if err := c.directoryWatcher(ctx, cfg, cfg.CNINetDir, cfgFile); err != nil { + c.logger.Error("error with directory watcher", "error", err) + errCh <- err + } + }() + + // Wait for either a shutdown signal or an error from watchers + var responseCode int + select { + case sig := <-c.sigCh: + c.logger.Info("Received shutdown signal", "signal", sig) + responseCode = 0 + case err := <-errCh: + c.logger.Error("Received error from watcher", "error", err) + responseCode = 1 + // Cancel context to stop other goroutines } - return 0 + cancel() + wg.Wait() + // wait for watchers to finish as they regenerate pluginconfs/tokens/kubeconfigs + c.cleanup(cfg, cfgFile) + return responseCode } // cleanup removes the consul-cni configuration, kubeconfig and cni-host-token- file from cniNetDir and cniBinDir. func (c *Command) cleanup(cfg *config.CNIConfig, cfgFile string) { var err error c.logger.Info("Shutdown received, cleaning up") + // Its important to cleanup in this order as plugin conf binds cni in the workflow if cfgFile != "" { err = removeCNIConfig(cfgFile) if err != nil { c.logger.Error("Unable to cleanup CNI Config: %w", err) } } - c.logger.Info("Removing file", "file", cfg.CNIHostTokenPath) + cniBinaryPath := filepath.Join(cfg.CNIBinDir, consulCNIName) + c.logger.Info("Removing file", "file", cniBinaryPath) + err = removeFile(cniBinaryPath) + if err != nil { + c.logger.Error("Unable to remove %s file: %w", cniBinaryPath, err) + } + + c.logger.Info("Removing file", "file", cfg.CNIHostTokenPath) err = removeFile(cfg.CNIHostTokenPath) if err != nil { c.logger.Error("Unable to remove %s file: %w", cfg.CNIHostTokenPath, err) } + kubeconfig := filepath.Join(cfg.CNINetDir, cfg.Kubeconfig) c.logger.Info("Removing file", "file", kubeconfig) @@ -256,12 +290,13 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d } defer func() { _ = watcher.Close() - c.cleanup(cfg, cfgFile) }() // Generate the initial kubeconfig file that will be used by the plugin to communicate with the kubernetes api. c.logger.Info("Creating kubeconfig", "file", cfg.Kubeconfig) kubeConfigFile := filepath.Join(cfg.CNINetDir, cfg.Kubeconfig) + cniBinaryPath := filepath.Join(cfg.CNIBinDir, consulCNIName) + cniBinarySourcePath := filepath.Join(c.flagCNIBinSourceDir, consulCNIName) err = createKubeConfig(cfg) if err != nil { c.logger.Error("could not create kube config", "error", err) @@ -281,9 +316,33 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d // This directory watcher doesn't need to handle anything related to token if strings.Contains(event.Name, config.DefaultCNIHostTokenFilename) { c.logger.Info("Skipping event for host token path. Nothing to do.", "event_path", event.Name) - break + continue } + // older daemonset can delete this token on SIGTERM as cleanup + if cfg.AutorotateToken && event.Name == cfg.CNIHostTokenPath { + if event.Op&fsnotify.Remove != 0 { + c.logger.Info("Creating host token", "file", cfg.CNIHostTokenPath) + err := copyToken(cfg.CNITokenPath, cfg.CNIHostTokenPath) + if err != nil { + c.logger.Error("could not create host token", "error", err) + return err + } + } + continue + } + // older daemonset can delete this binary on SIGTERM as cleanup + if event.Name == cniBinaryPath { + if event.Op&fsnotify.Remove != 0 { + c.logger.Info("Creating CNI binary", "file", cniBinaryPath) + err := copyFile(cniBinarySourcePath, cniBinaryPath) + if err != nil { + c.logger.Error("could not create cni binary", "error", err) + return err + } + } + continue + } // older daemonset can delete this kubeconfig on SIGTERM as cleanup // new pod should listen to remove and regenerate it. if event.Name == kubeConfigFile { @@ -292,11 +351,12 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d err := createKubeConfig(cfg) if err != nil { c.logger.Error("could not create kube config", "error", err) - break + return err } } - break + continue } + // Only repair things if this is a non-multus setup. Multus config is handled differently // than chained plugins if !cfg.Multus { @@ -306,14 +366,14 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d cfgFile, err = defaultCNIConfigFile(dir) if err != nil { c.logger.Error("Unable get default config file", "error", err) - break + return err } if strings.HasSuffix(cfgFile, ".conf") { cfgFile, err = confListFileFromConfFile(cfgFile) if err != nil { c.logger.Error("could convert .conf file to .conflist file", "error", err) - break + return err } } @@ -331,19 +391,18 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d } } else { c.logger.Info("Valid config file detected, nothing to do") - break + continue } } } } + case err, ok := <-watcher.Errors: if !ok { c.logger.Error("Event watcher event is not ok", "error", err) } case <-ctx.Done(): return nil - case <-c.sigCh: - return nil } } } @@ -366,12 +425,8 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok return fmt.Errorf("could not watch token file %s: %w", sourceTokenPath, err) } - // Copy token if initial access is possible - if _, err := os.Stat(sourceTokenPath); err == nil { - c.logger.Info("Initial token copy to host", "sourcePath", sourceTokenPath, "destinationPath", hostTokenPath) - if err := copyToken(sourceTokenPath, hostTokenPath); err != nil { - c.logger.Error("Failed initial token copy", "error", err) - } + if err := copyToken(sourceTokenPath, hostTokenPath); err != nil { + c.logger.Info("Failed to perform initial copy token", "error", err) } for { @@ -379,7 +434,7 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok case event, ok := <-watcher.Events: if !ok { c.logger.Error("Token watcher event is not ok", "event", event) - break + return fmt.Errorf("token watcher event channel closed unexpectedly") } // Only handle events for the specific token file @@ -389,7 +444,7 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok if event.Name != sourceTokenPath { c.logger.Info("Skipping event as it's not for the source token path", "event_path", event.Name, "source_token_path", sourceTokenPath) - break + continue } // Handle Write event on symlink update to point to a new file // but the symlink's creation timestamp changes on doing such update as well. @@ -397,41 +452,44 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok if event.Op&(fsnotify.Remove|fsnotify.Chmod) != 0 { // Re-add watcher after remove/chmod backoff := time.Second - for i := 0; i < 5; i++ { + waitCount := 5 + for i := 1; i <= waitCount; i++ { if err := watcher.Add(sourceTokenPath); err != nil { c.logger.Error("Failed to re-add watcher after remove/chmod", "error", err, "attempt", i+1) time.Sleep(backoff) backoff *= 2 + if waitCount == i { + return fmt.Errorf("failed to re-add watcher after remove/chmod after %d attempts", waitCount) + } continue } - break } - if _, err := os.Stat(sourceTokenPath); err == nil { - if err := copyToken(sourceTokenPath, hostTokenPath); err != nil { - c.logger.Error("Failed to copy token after symlink update", "error", err) - } else { - c.logger.Info("Successfully copied new token after symlink update") - } + + if err := copyToken(sourceTokenPath, hostTokenPath); err != nil { + c.logger.Error("Failed to copy token after symlink update", "error", err) + } else { + c.logger.Debug("Successfully copied new token after symlink update") } } case err, ok := <-watcher.Errors: if !ok { c.logger.Error("Token watcher error channel closed") - continue + return fmt.Errorf("token watcher error channel closed unexpectedly") } c.logger.Error("Token watcher error", "error", err) case <-ctx.Done(): return nil - case <-c.sigCh: - return nil } } } func copyToken(src, dst string) error { // Read source token + if _, err := os.Stat(src); err == nil { + return err + } content, err := os.ReadFile(src) if err != nil { return fmt.Errorf("failed to read source token: %w", err) From cc76ab3d774c81c600423e0236c1c9e30eabfa36 Mon Sep 17 00:00:00 2001 From: Mukul Anand Date: Sat, 6 Sep 2025 21:00:13 +0000 Subject: [PATCH 2/4] backport of commit a49f1e8c123b9adfae67830d821f88260c17fe10 --- .../subcommand/install-cni/command.go | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/control-plane/subcommand/install-cni/command.go b/control-plane/subcommand/install-cni/command.go index 9bb9005248..c2aee501df 100644 --- a/control-plane/subcommand/install-cni/command.go +++ b/control-plane/subcommand/install-cni/command.go @@ -87,7 +87,7 @@ func (c *Command) init() { // Run runs the command. func (c *Command) Run(args []string) int { c.once.Do(c.init) - +initialize: if err := c.flagSet.Parse(args); err != nil { return 1 } @@ -223,21 +223,24 @@ func (c *Command) Run(args []string) int { }() // Wait for either a shutdown signal or an error from watchers - var responseCode int + var reRun bool = false select { case sig := <-c.sigCh: c.logger.Info("Received shutdown signal", "signal", sig) - responseCode = 0 case err := <-errCh: c.logger.Error("Received error from watcher", "error", err) - responseCode = 1 + reRun = true + //re-run this command to fix the issue // Cancel context to stop other goroutines } cancel() wg.Wait() + if reRun { + goto initialize + } // wait for watchers to finish as they regenerate pluginconfs/tokens/kubeconfigs c.cleanup(cfg, cfgFile) - return responseCode + return 0 } // cleanup removes the consul-cni configuration, kubeconfig and cni-host-token- file from cniNetDir and cniBinDir. @@ -313,12 +316,6 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d // created before other CNI plugins were installed. if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 { // Separate tokenFileWatcher updates the token file in the host path - // This directory watcher doesn't need to handle anything related to token - if strings.Contains(event.Name, config.DefaultCNIHostTokenFilename) { - c.logger.Info("Skipping event for host token path. Nothing to do.", "event_path", event.Name) - continue - } - // older daemonset can delete this token on SIGTERM as cleanup if cfg.AutorotateToken && event.Name == cfg.CNIHostTokenPath { if event.Op&fsnotify.Remove != 0 { @@ -329,8 +326,9 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d return err } } - continue + break } + // older daemonset can delete this binary on SIGTERM as cleanup if event.Name == cniBinaryPath { if event.Op&fsnotify.Remove != 0 { @@ -341,8 +339,9 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d return err } } - continue + break } + // older daemonset can delete this kubeconfig on SIGTERM as cleanup // new pod should listen to remove and regenerate it. if event.Name == kubeConfigFile { @@ -354,7 +353,7 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d return err } } - continue + break } // Only repair things if this is a non-multus setup. Multus config is handled differently @@ -391,7 +390,6 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d } } else { c.logger.Info("Valid config file detected, nothing to do") - continue } } } @@ -444,7 +442,7 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok if event.Name != sourceTokenPath { c.logger.Info("Skipping event as it's not for the source token path", "event_path", event.Name, "source_token_path", sourceTokenPath) - continue + break } // Handle Write event on symlink update to point to a new file // but the symlink's creation timestamp changes on doing such update as well. @@ -461,14 +459,13 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok if waitCount == i { return fmt.Errorf("failed to re-add watcher after remove/chmod after %d attempts", waitCount) } - continue } } if err := copyToken(sourceTokenPath, hostTokenPath); err != nil { c.logger.Error("Failed to copy token after symlink update", "error", err) } else { - c.logger.Debug("Successfully copied new token after symlink update") + c.logger.Info("Successfully copied new token from source") } } From e8db600fdb9d33a5b63fda68700926f01349de4b Mon Sep 17 00:00:00 2001 From: Mukul Anand Date: Sat, 6 Sep 2025 22:09:12 +0000 Subject: [PATCH 3/4] backport of commit 19a79f72a3bbe7c5b2ef095bad60a2c06fbcf71a --- .../subcommand/install-cni/command_test.go | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/control-plane/subcommand/install-cni/command_test.go b/control-plane/subcommand/install-cni/command_test.go index 34a81bd280..c8df22be62 100644 --- a/control-plane/subcommand/install-cni/command_test.go +++ b/control-plane/subcommand/install-cni/command_test.go @@ -105,6 +105,172 @@ func TestRun_DirectoryWatcher(t *testing.T) { time.Sleep(50 * time.Millisecond) } +func TestRun_FileRegenerationAfterRemoval(t *testing.T) { + tests := []struct { + name string + autorotateToken bool + fileToRemove string + setupFunc func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) + verifyFunc func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) + }{ + { + name: "cni-host-token regeneration after removal", + autorotateToken: true, + fileToRemove: "cni-host-token", + setupFunc: func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) { + // Create initial host token file + hostTokenPath := cfg.CNIHostTokenPath + err := os.WriteFile(hostTokenPath, []byte("initial-token-content"), 0644) + if err != nil { + return "", err + } + // Create source token file that will be copied + err = os.WriteFile(cfg.CNITokenPath, []byte("updated-token-content"), 0644) + if err != nil { + return "", err + } + return hostTokenPath, nil + }, + verifyFunc: func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) { + // Verify the host token file was regenerated + retry.Run(t, func(r *retry.R) { + _, err := os.Stat(cfg.CNIHostTokenPath) + require.NoError(r, err, "Host token file should be regenerated") + }) + // Verify content was copied from source token + content, err := os.ReadFile(cfg.CNIHostTokenPath) + require.NoError(t, err) + require.Equal(t, "updated-token-content", string(content)) + }, + }, + { + name: "cni binary regeneration after removal", + autorotateToken: false, + fileToRemove: "consul-cni", + setupFunc: func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) { + // Create initial CNI binary + cniBinaryPath := filepath.Join() + err := os.WriteFile(cniBinaryPath, []byte("initial-binary-content"), 0755) + if err != nil { + return "", err + } + return cniBinaryPath, nil + }, + verifyFunc: func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) { + // Verify the CNI binary was regenerated + cniBinaryPath := filepath.Join(binDir, consulCNIName) + retry.Run(t, func(r *retry.R) { + _, err := os.Stat(cniBinaryPath) + require.NoError(r, err, "CNI binary should be regenerated") + }) + // Verify content was copied from source + content, err := os.ReadFile(cniBinaryPath) + require.NoError(t, err) + require.Equal(t, "source-binary-content", string(content)) + }, + }, + { + name: "kubeconfig file regeneration after removal", + autorotateToken: false, + fileToRemove: "ZZZ-consul-cni-kubeconfig", + setupFunc: func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) { + // Create initial kubeconfig file + kubeconfigPath := filepath.Join(tempDir, cfg.Kubeconfig) + err := os.WriteFile(kubeconfigPath, []byte("initial-kubeconfig-content"), 0644) + if err != nil { + return "", err + } + return kubeconfigPath, nil + }, + verifyFunc: func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) { + // Verify the kubeconfig file was regenerated + kubeconfigPath := filepath.Join(tempDir, cfg.Kubeconfig) + retry.Run(t, func(r *retry.R) { + _, err := os.Stat(kubeconfigPath) + require.NoError(r, err, "Kubeconfig file should be regenerated") + }) + // Verify file is not empty (createKubeConfig should have generated content) + info, err := os.Stat(kubeconfigPath) + require.NoError(t, err) + require.Greater(t, info.Size(), int64(0), "Kubeconfig should not be empty") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temp directories + tempDir := t.TempDir() + binDir := t.TempDir() + sourceDir := t.TempDir() + + // Create source CNI binary for copying + sourceBinaryPath := filepath.Join(sourceDir, consulCNIName) + err := os.WriteFile(sourceBinaryPath, []byte("source-binary-content"), 0755) + require.NoError(t, err) + + // Create a default configuration + uid := fmt.Sprintf("%d", time.Now().UnixNano()) + cfg := &config.CNIConfig{ + Name: config.DefaultPluginName, + Type: config.DefaultPluginType, + CNITokenPath: filepath.Join(tempDir, "token"), + CNIHostTokenPath: func() string { + if tt.autorotateToken { + return filepath.Join(tempDir, config.DefaultCNIHostTokenFilename+"-"+uid) + } + return "" + }(), + AutorotateToken: tt.autorotateToken, + CNIBinDir: binDir, + CNINetDir: tempDir, + Kubeconfig: "ZZZ-consul-cni-kubeconfig", + LogLevel: "info", + Multus: false, + } + + // Setup the Command + ui := cli.NewMockUi() + cmd := &Command{ + UI: ui, + flagCNIBinSourceDir: sourceDir, + } + cmd.init() + cmd.logger, err = common.Logger("info", false) + require.NoError(t, err) + + // Setup initial files + removedFilePath, err := tt.setupFunc(tempDir, binDir, cfg) + require.NoError(t, err) + + // Create context and start directory watcher + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + go func() { + err := cmd.directoryWatcher(ctx, cfg, tempDir, "") + if err != nil && ctx.Err() == nil { + t.Errorf("directoryWatcher error: %v", err) + } + }() + + // Wait for watcher to initialize + time.Sleep(100 * time.Millisecond) + + // Remove the file to trigger regeneration + t.Logf("Removing file: %s", removedFilePath) + err = os.Remove(removedFilePath) + require.NoError(t, err) + + // Wait for file system event to be processed + time.Sleep(200 * time.Millisecond) + + // Verify file regeneration + tt.verifyFunc(t, tempDir, binDir, cfg, removedFilePath) + }) + } +} + func replaceFile(srcFile, destFile string) error { if _, err := os.Stat(srcFile); os.IsNotExist(err) { return fmt.Errorf("source %s file does not exist: %v", srcFile, err) From ec955fac4a95f9a3127b9b4ca334b8d9e6eb142f Mon Sep 17 00:00:00 2001 From: Mukul Anand Date: Sun, 7 Sep 2025 14:17:44 +0000 Subject: [PATCH 4/4] backport of commit 136f8abcd33e7f52b50094632411180c2de7602a --- .../subcommand/install-cni/binary.go | 7 +- .../subcommand/install-cni/binary_test.go | 6 +- .../subcommand/install-cni/cniconfig.go | 2 +- .../subcommand/install-cni/cniconfig_test.go | 25 +- .../subcommand/install-cni/command.go | 138 +++--- .../subcommand/install-cni/command_test.go | 406 +++++++++++++----- .../subcommand/install-cni/kubeconfig.go | 4 +- 7 files changed, 401 insertions(+), 187 deletions(-) diff --git a/control-plane/subcommand/install-cni/binary.go b/control-plane/subcommand/install-cni/binary.go index 5951df665a..4972f8fb81 100644 --- a/control-plane/subcommand/install-cni/binary.go +++ b/control-plane/subcommand/install-cni/binary.go @@ -10,14 +10,15 @@ import ( ) // copyFile copies a file from a source directory to a destination directory. -func copyFile(srcFile, destDir string) error { +func copyFile(srcFile, destDir, filename string) error { // If the src file does not exist then either the incorrect command line argument was used or // the docker container we built is broken somehow. if _, err := os.Stat(srcFile); os.IsNotExist(err) { return err } - - filename := filepath.Base(srcFile) + if filename == "" { + filename = filepath.Base(srcFile) + } // If the destDir does not exist then the incorrect command line argument was used or // the CNI settings for the kubelet are not correct. info, err := os.Stat(destDir) diff --git a/control-plane/subcommand/install-cni/binary_test.go b/control-plane/subcommand/install-cni/binary_test.go index 397751d42c..8acc87610f 100644 --- a/control-plane/subcommand/install-cni/binary_test.go +++ b/control-plane/subcommand/install-cni/binary_test.go @@ -76,7 +76,7 @@ func TestCopyFile(t *testing.T) { name: "cannot read source file", srcFile: func() string { tempDir := t.TempDir() - err := copyFile("testdata/10-kindnet.conflist", tempDir) + err := copyFile("testdata/10-kindnet.conflist", tempDir, "") require.NoError(t, err) filepath := filepath.Join(tempDir, "10-kindnet.conflist") os.Chmod(filepath, 0111) @@ -108,7 +108,7 @@ func TestCopyFile(t *testing.T) { t.Run(c.name, func(t *testing.T) { destDir := c.dir() srcFile := c.srcFile() - actualErr := copyFile(srcFile, destDir) + actualErr := copyFile(srcFile, destDir, "") expErr := c.expectedErr(srcFile, destDir) @@ -133,7 +133,7 @@ func TestRemoveFile(t *testing.T) { name: "can remove file", srcFile: func() string { tempDir := t.TempDir() - err := copyFile("testdata/10-kindnet.conflist", tempDir) + err := copyFile("testdata/10-kindnet.conflist", tempDir, "") require.NoError(t, err) filepath := filepath.Join(tempDir, "10-kindnet.conflist") return filepath diff --git a/control-plane/subcommand/install-cni/cniconfig.go b/control-plane/subcommand/install-cni/cniconfig.go index 448c9efa92..5ad64d509c 100644 --- a/control-plane/subcommand/install-cni/cniconfig.go +++ b/control-plane/subcommand/install-cni/cniconfig.go @@ -291,7 +291,7 @@ func validConfig(cfg *config.CNIConfig, cfgFile string) error { if !ok { return fmt.Errorf("error reading plugin from plugin list") } - if plugin["type"] == consulCNIName { + if plugin["type"] == cfg.Type { // Populate existingCfg with the consul-cni plugin info so that we can compare it with what // is expected. err := mapstructure.Decode(plugin, &existingCfg) diff --git a/control-plane/subcommand/install-cni/cniconfig_test.go b/control-plane/subcommand/install-cni/cniconfig_test.go index 3d4e14aa69..bee628c36c 100644 --- a/control-plane/subcommand/install-cni/cniconfig_test.go +++ b/control-plane/subcommand/install-cni/cniconfig_test.go @@ -40,7 +40,7 @@ func TestDefaultCNIConfigFile(t *testing.T) { cfgFile: "testdata/10-kindnet.conflist", dir: func(cfgFile string) string { tempDir := t.TempDir() - err := copyFile(cfgFile, tempDir) + err := copyFile(cfgFile, tempDir, "") if err != nil { t.Fatal(err) } @@ -54,11 +54,11 @@ func TestDefaultCNIConfigFile(t *testing.T) { cfgFile: "testdata/10-kindnet.conflist", dir: func(cfgFile string) string { tempDir := t.TempDir() - err := copyFile(cfgFile, tempDir) + err := copyFile(cfgFile, tempDir, "") if err != nil { t.Fatal(err) } - err = copyFile("testdata/10-fake-cni.conf", tempDir) + err = copyFile("testdata/10-fake-cni.conf", tempDir, "") if err != nil { t.Fatal(err) } @@ -87,7 +87,7 @@ func TestConfListFromConfFile(t *testing.T) { expectedCfgFile := "testdata/00-chained-plugins.conflist" tempDir := t.TempDir() - err := copyFile(cfgFile, tempDir) + err := copyFile(cfgFile, tempDir, "") require.NoError(t, err) filename := filepath.Base(cfgFile) @@ -167,7 +167,7 @@ func TestAppendCNIConfig(t *testing.T) { t.Run(c.name, func(t *testing.T) { // Copy the config file to a temporary location so that we can append to it. tempDir := t.TempDir() - err := copyFile(c.cfgFile, tempDir) + err := copyFile(c.cfgFile, tempDir, "") require.NoError(t, err) // Get the config file name in the tempdir. @@ -206,7 +206,7 @@ func TestConfigFileToMap(t *testing.T) { } tempDir := t.TempDir() - err := copyFile(cfgFile, tempDir) + err := copyFile(cfgFile, tempDir, "") require.NoError(t, err) filename := filepath.Base(cfgFile) @@ -296,7 +296,7 @@ func TestRemoveCNIConfig(t *testing.T) { t.Run(c.name, func(t *testing.T) { // copy the config file to a temporary location so that we can append to it tempDir := t.TempDir() - err := copyFile(c.goldenFile, tempDir) + err := copyFile(c.goldenFile, tempDir, "") if err != nil { t.Fatal(err) } @@ -336,10 +336,12 @@ func TestValidConfig(t *testing.T) { expectedErr: fmt.Errorf("consul-cni config missing from config file"), }, { - name: "config passed to installer does not match config in config file", - cfgFile: "testdata/10-kindnet.conflist.golden", - consulConfig: &config.CNIConfig{}, - expectedErr: fmt.Errorf("consul-cni config has changed"), + name: "config passed to installer does not match config in config file", + cfgFile: "testdata/10-kindnet.conflist.golden", + consulConfig: &config.CNIConfig{ + Type: consulCNIName, + }, + expectedErr: fmt.Errorf("consul-cni config has changed"), }, { name: "config passed to installer does not match config in config file", @@ -347,6 +349,7 @@ func TestValidConfig(t *testing.T) { consulConfig: &config.CNIConfig{ CNIBinDir: "foo", CNINetDir: "bar", + Type: consulCNIName, }, expectedErr: fmt.Errorf("consul-cni config has changed"), }, diff --git a/control-plane/subcommand/install-cni/command.go b/control-plane/subcommand/install-cni/command.go index c2aee501df..5a5ee0b2c5 100644 --- a/control-plane/subcommand/install-cni/command.go +++ b/control-plane/subcommand/install-cni/command.go @@ -25,9 +25,10 @@ import ( ) const ( - defaultCNIBinSourceDir = "/bin" - consulCNIName = "consul-cni" // Name of the plugin and binary. They must be the same as per the CNI spec. - defaultLogJSON = false + defaultCNIBinSourceDir = "/bin" + consulCNIName = "consul-cni" // Name of the plugin and binary. They must be the same as per the CNI spec. + defaultLogJSON = false + maxInitializeRetryCount = 3 ) // Command flags and structure. @@ -51,6 +52,10 @@ type Command struct { flagLogJSON bool // flagMultus is a boolean flag for multus support. flagMultus bool + // flagInstallationID is a unique identifier for this installation instance. + flagInstallationID string + // flagCNITokenPath is the path to the CNI token file for testing purposes. + flagCNITokenPath string flagSet *flag.FlagSet @@ -72,6 +77,8 @@ func (c *Command) init() { c.flagSet.BoolVar(&c.flagK8sAutorotateToken, "autorotate-token", config.DefaultAutorotateToken, "Enable or disable token autorotate feature.") c.flagSet.BoolVar(&c.flagLogJSON, "log-json", defaultLogJSON, "Enable or disable JSON output format for logging.") c.flagSet.BoolVar(&c.flagMultus, "multus", config.DefaultMultus, "If the plugin is a multus plugin (default = false)") + c.flagSet.StringVar(&c.flagInstallationID, "installation-id", "", "Unique identifier for this installation instance (auto-generated if not provided)") + c.flagSet.StringVar(&c.flagCNITokenPath, "cni-token-path", "", "Path to the CNI token file for testing purposes.") c.help = flags.Usage(help, c.flagSet) @@ -87,6 +94,7 @@ func (c *Command) init() { // Run runs the command. func (c *Command) Run(args []string) int { c.once.Do(c.init) + var tryCount = 1 initialize: if err := c.flagSet.Parse(args); err != nil { return 1 @@ -102,22 +110,35 @@ initialize: } } - uid := fmt.Sprintf("%d", time.Now().UnixNano()) + // Generate or use provided installation ID + var installationID string + if c.flagInstallationID != "" { + installationID = c.flagInstallationID + } else { + installationID = fmt.Sprintf("%d", time.Now().UnixNano()) + } + // Create the CNI Config from command flags. cfg := &config.CNIConfig{ - Name: config.DefaultPluginName, - Type: config.DefaultPluginType, - CNITokenPath: config.DefaultCNITokenDir + "/" + config.DefaultCNITokenFilename, + Name: config.DefaultPluginName, + Type: config.DefaultPluginType + "-" + installationID, + CNITokenPath: func() string { + dir := c.flagCNITokenPath + if dir == "" { + dir = config.DefaultCNITokenDir + } + return filepath.Join(dir, config.DefaultCNITokenFilename) + }(), CNIHostTokenPath: func() string { if c.flagK8sAutorotateToken { - return c.flagCNINetDir + "/" + config.DefaultCNIHostTokenFilename + "-" + uid + return filepath.Join(c.flagCNINetDir, config.DefaultCNIHostTokenFilename+"-"+installationID) } return "" }(), AutorotateToken: c.flagK8sAutorotateToken, CNIBinDir: c.flagCNIBinDir, CNINetDir: c.flagCNINetDir, - Kubeconfig: c.flagKubeconfig, + Kubeconfig: c.flagKubeconfig + "-" + installationID, LogLevel: c.flagLogLevel, Multus: c.flagMultus, } @@ -141,7 +162,9 @@ initialize: // Copy the consul-cni binary from the installer container to the host. c.logger.Info("Copying consul-cni binary", "destination", cfg.CNIBinDir) srcFile := filepath.Join(c.flagCNIBinSourceDir, consulCNIName) - err := copyFile(srcFile, cfg.CNIBinDir) + + //type is what the kubelet tries to lookup as filename in cniNetDir + err := copyFile(srcFile, cfg.CNIBinDir, cfg.Type) if err != nil { c.logger.Error("could not copy consul-cni binary", "error", err) return 1 @@ -196,6 +219,7 @@ initialize: errCh := make(chan error, 2) var wg sync.WaitGroup + // watch for changes in the default cni serviceaccount token directory if cfg.AutorotateToken { // if autorotate-token is enabled, we need to watch the token file for changes and copy it to the host @@ -212,7 +236,7 @@ initialize: }() } - // Watch for changes in the cniNetDir directory and fix/install the config file if need be. + // Watch for changes in the cniNetDir directory and fix/install the config files if need be. go func() { wg.Add(1) defer wg.Done() @@ -229,13 +253,17 @@ initialize: c.logger.Info("Received shutdown signal", "signal", sig) case err := <-errCh: c.logger.Error("Received error from watcher", "error", err) - reRun = true - //re-run this command to fix the issue - // Cancel context to stop other goroutines + if tryCount < maxInitializeRetryCount { + reRun = true + tryCount++ + } else { + return 1 + } } cancel() wg.Wait() if reRun { + c.cleanup(cfg, "") // do soft cleanup of artifacts and re-initialize goto initialize } // wait for watchers to finish as they regenerate pluginconfs/tokens/kubeconfigs @@ -255,7 +283,7 @@ func (c *Command) cleanup(cfg *config.CNIConfig, cfgFile string) { } } - cniBinaryPath := filepath.Join(cfg.CNIBinDir, consulCNIName) + cniBinaryPath := filepath.Join(cfg.CNIBinDir, cfg.Type) c.logger.Info("Removing file", "file", cniBinaryPath) err = removeFile(cniBinaryPath) if err != nil { @@ -283,9 +311,8 @@ func (c *Command) cleanup(cfg *config.CNIConfig, cfgFile string) { func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, dir, cfgFile string) error { watcher, err := fsnotify.NewWatcher() if err != nil { - return fmt.Errorf("could not create watcher: %w", err) + return fmt.Errorf("could not create dirwatcher: %w", err) } - c.logger.Info("Creating directory watcher for", "directory", dir) err = watcher.Add(dir) if err != nil { @@ -297,10 +324,8 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d // Generate the initial kubeconfig file that will be used by the plugin to communicate with the kubernetes api. c.logger.Info("Creating kubeconfig", "file", cfg.Kubeconfig) - kubeConfigFile := filepath.Join(cfg.CNINetDir, cfg.Kubeconfig) - cniBinaryPath := filepath.Join(cfg.CNIBinDir, consulCNIName) - cniBinarySourcePath := filepath.Join(c.flagCNIBinSourceDir, consulCNIName) - err = createKubeConfig(cfg) + kubeConfigFile := filepath.Join(dir, cfg.Kubeconfig) + err = createKubeConfig(dir, cfg) if err != nil { c.logger.Error("could not create kube config", "error", err) } @@ -317,37 +342,17 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 { // Separate tokenFileWatcher updates the token file in the host path // older daemonset can delete this token on SIGTERM as cleanup - if cfg.AutorotateToken && event.Name == cfg.CNIHostTokenPath { - if event.Op&fsnotify.Remove != 0 { - c.logger.Info("Creating host token", "file", cfg.CNIHostTokenPath) - err := copyToken(cfg.CNITokenPath, cfg.CNIHostTokenPath) - if err != nil { - c.logger.Error("could not create host token", "error", err) - return err - } - } + if event.Name == cfg.CNIHostTokenPath { + c.logger.Info("Token file updated", "file", event.Name) break } - - // older daemonset can delete this binary on SIGTERM as cleanup - if event.Name == cniBinaryPath { - if event.Op&fsnotify.Remove != 0 { - c.logger.Info("Creating CNI binary", "file", cniBinaryPath) - err := copyFile(cniBinarySourcePath, cniBinaryPath) - if err != nil { - c.logger.Error("could not create cni binary", "error", err) - return err - } - } - break - } - // older daemonset can delete this kubeconfig on SIGTERM as cleanup // new pod should listen to remove and regenerate it. + // currently this is not unit-testable as createKubeConfig is not mockable if event.Name == kubeConfigFile { if event.Op&fsnotify.Remove != 0 { c.logger.Info("Creating kubeconfig", "file", cfg.Kubeconfig) - err := createKubeConfig(cfg) + err := createKubeConfig(dir, cfg) if err != nil { c.logger.Error("could not create kube config", "error", err) return err @@ -408,18 +413,19 @@ func (c *Command) directoryWatcher(ctx context.Context, cfg *config.CNIConfig, d // In case of autorotate-token, we are using projected tokens which doesn't support hostpath mount, // we need to watch the token file for changes and copy it to the host. func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTokenPath string) error { - watcher, err := fsnotify.NewWatcher() + sourceTokenWatcher, err := fsnotify.NewWatcher() if err != nil { - return fmt.Errorf("could not create token watcher: %w", err) + return fmt.Errorf("could not create sourcetokenWatcher: %w", err) + } + if err != nil { + return fmt.Errorf("could not create desttokenWatcher: %w", err) } - defer func() { - _ = watcher.Close() + _ = sourceTokenWatcher.Close() }() - // Watch the directory containing the token instead of the symlink - c.logger.Info("Creating token watcher for", "file", sourceTokenPath) - if err := watcher.Add(sourceTokenPath); err != nil { + c.logger.Info("Creating sourceTokenWatcher for", "file", sourceTokenPath) + if err := sourceTokenWatcher.Add(sourceTokenPath); err != nil { return fmt.Errorf("could not watch token file %s: %w", sourceTokenPath, err) } @@ -429,10 +435,10 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok for { select { - case event, ok := <-watcher.Events: + case event, ok := <-sourceTokenWatcher.Events: if !ok { - c.logger.Error("Token watcher event is not ok", "event", event) - return fmt.Errorf("token watcher event channel closed unexpectedly") + c.logger.Error("Token sourceTokenWatcher event is not ok", "event", event) + return fmt.Errorf("token sourceTokenWatcher event channel closed unexpectedly") } // Only handle events for the specific token file @@ -440,24 +446,20 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok "event_type", event.Op.String(), "file", event.Name) - if event.Name != sourceTokenPath { - c.logger.Info("Skipping event as it's not for the source token path", "event_path", event.Name, "source_token_path", sourceTokenPath) - break - } // Handle Write event on symlink update to point to a new file // but the symlink's creation timestamp changes on doing such update as well. if event.Op&(fsnotify.Remove|fsnotify.Chmod) != 0 { - // Re-add watcher after remove/chmod + // Re-add sourceTokenWatcher after remove/chmod backoff := time.Second waitCount := 5 for i := 1; i <= waitCount; i++ { - if err := watcher.Add(sourceTokenPath); err != nil { - c.logger.Error("Failed to re-add watcher after remove/chmod", "error", err, "attempt", i+1) + if err := sourceTokenWatcher.Add(sourceTokenPath); err != nil { + c.logger.Error("Failed to re-add sourceTokenWatcher after remove/chmod", "error", err, "attempt", i+1) time.Sleep(backoff) backoff *= 2 if waitCount == i { - return fmt.Errorf("failed to re-add watcher after remove/chmod after %d attempts", waitCount) + return fmt.Errorf("failed to re-add sourceTokenWatcher after remove/chmod after %d attempts", waitCount) } } } @@ -469,12 +471,12 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok } } - case err, ok := <-watcher.Errors: + case err, ok := <-sourceTokenWatcher.Errors: if !ok { - c.logger.Error("Token watcher error channel closed") - return fmt.Errorf("token watcher error channel closed unexpectedly") + c.logger.Error("SourceTokenWatcher error channel closed") + return fmt.Errorf("SourceTokenWatcher error channel closed unexpectedly") } - c.logger.Error("Token watcher error", "error", err) + c.logger.Error("SourceTokenWatcher error", "error", err) case <-ctx.Done(): return nil @@ -484,7 +486,7 @@ func (c *Command) tokenFileWatcher(ctx context.Context, sourceTokenPath, hostTok func copyToken(src, dst string) error { // Read source token - if _, err := os.Stat(src); err == nil { + if _, err := os.Stat(src); err != nil { return err } content, err := os.ReadFile(src) diff --git a/control-plane/subcommand/install-cni/command_test.go b/control-plane/subcommand/install-cni/command_test.go index c8df22be62..0f3ca464f7 100644 --- a/control-plane/subcommand/install-cni/command_test.go +++ b/control-plane/subcommand/install-cni/command_test.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "syscall" "testing" "time" @@ -62,7 +63,7 @@ func TestRun_DirectoryWatcher(t *testing.T) { time.Sleep(50 * time.Millisecond) t.Log("File event 1: Copy a base config file that does not contain the consul entry. Should detect and add consul-cni") - err = copyFile(baseConfigFile, tempDir) + err = copyFile(baseConfigFile, tempDir, "") require.NoError(t, err) time.Sleep(50 * time.Millisecond) // The golden file contains the consul config. @@ -105,95 +106,207 @@ func TestRun_DirectoryWatcher(t *testing.T) { time.Sleep(50 * time.Millisecond) } -func TestRun_FileRegenerationAfterRemoval(t *testing.T) { +func TestRun_TokenFileWatcher(t *testing.T) { tests := []struct { - name string - autorotateToken bool - fileToRemove string - setupFunc func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) - verifyFunc func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) + name string + description string + testFunc func(t *testing.T, cmd *Command, sourceTokenPath, hostTokenPath string, ctx context.Context) }{ { - name: "cni-host-token regeneration after removal", - autorotateToken: true, - fileToRemove: "cni-host-token", - setupFunc: func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) { - // Create initial host token file - hostTokenPath := cfg.CNIHostTokenPath - err := os.WriteFile(hostTokenPath, []byte("initial-token-content"), 0644) - if err != nil { - return "", err - } - // Create source token file that will be copied - err = os.WriteFile(cfg.CNITokenPath, []byte("updated-token-content"), 0644) - if err != nil { - return "", err - } - return hostTokenPath, nil - }, - verifyFunc: func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) { - // Verify the host token file was regenerated + name: "source token file deletion and recreation", + description: "Test that when source token file is deleted and recreated with new content, host file reflects updated content", + testFunc: func(t *testing.T, cmd *Command, sourceTokenPath, hostTokenPath string, ctx context.Context) { + // Create initial source token file + initialContent := "initial-token-content-12345" + err := os.WriteFile(sourceTokenPath, []byte(initialContent), 0644) + require.NoError(t, err) + + // Start token file watcher + go func() { + err := cmd.tokenFileWatcher(ctx, sourceTokenPath, hostTokenPath) + if err != nil && ctx.Err() == nil { + t.Errorf("tokenFileWatcher error: %v", err) + } + }() + + // Wait for initial copy + time.Sleep(200 * time.Millisecond) + + // Verify initial copy worked retry.Run(t, func(r *retry.R) { - _, err := os.Stat(cfg.CNIHostTokenPath) - require.NoError(r, err, "Host token file should be regenerated") + content, err := os.ReadFile(hostTokenPath) + require.NoError(r, err, "Host token file should exist") + require.Equal(r, initialContent, string(content), "Initial content should match") }) - // Verify content was copied from source token - content, err := os.ReadFile(cfg.CNIHostTokenPath) + + t.Log("Step 1: Delete source token file") + err = os.Remove(sourceTokenPath) require.NoError(t, err) - require.Equal(t, "updated-token-content", string(content)) + + // Wait for file system event processing + time.Sleep(100 * time.Millisecond) + + t.Log("Step 2: Recreate source token file with new content") + updatedContent := "updated-token-content-67890" + err = os.WriteFile(sourceTokenPath, []byte(updatedContent), 0644) + require.NoError(t, err) + + // Wait for token watcher to detect and copy new content + time.Sleep(300 * time.Millisecond) + + t.Log("Step 3: Verify host token file has updated content") + retry.Run(t, func(r *retry.R) { + content, err := os.ReadFile(hostTokenPath) + require.NoError(r, err, "Host token file should still exist") + require.Equal(r, updatedContent, string(content), "Host token should have updated content") + }) }, }, { - name: "cni binary regeneration after removal", - autorotateToken: false, - fileToRemove: "consul-cni", - setupFunc: func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) { - // Create initial CNI binary - cniBinaryPath := filepath.Join() - err := os.WriteFile(cniBinaryPath, []byte("initial-binary-content"), 0755) - if err != nil { - return "", err + name: "multiple source token updates", + description: "Test multiple consecutive updates to source token file are reflected in host file", + testFunc: func(t *testing.T, cmd *Command, sourceTokenPath, hostTokenPath string, ctx context.Context) { + // Create initial source token file + initialContent := "token-v1" + err := os.WriteFile(sourceTokenPath, []byte(initialContent), 0644) + require.NoError(t, err) + + // Start token file watcher + go func() { + err := cmd.tokenFileWatcher(ctx, sourceTokenPath, hostTokenPath) + if err != nil && ctx.Err() == nil { + t.Errorf("tokenFileWatcher error: %v", err) + } + }() + + // Wait for initial copy + time.Sleep(200 * time.Millisecond) + + // Test multiple updates + updates := []string{"token-v2", "token-v3", "token-v4"} + for i, content := range updates { + t.Logf("Step %d: Update token to %s", i+1, content) + + // Remove and recreate with new content + err = os.Remove(sourceTokenPath) + require.NoError(t, err) + time.Sleep(50 * time.Millisecond) + + err = os.WriteFile(sourceTokenPath, []byte(content), 0644) + require.NoError(t, err) + time.Sleep(200 * time.Millisecond) + + // Verify host file has the latest content + retry.Run(t, func(r *retry.R) { + hostContent, err := os.ReadFile(hostTokenPath) + require.NoError(r, err, "Host token file should exist") + require.Equal(r, content, string(hostContent), "Host token should have latest content") + }) } - return cniBinaryPath, nil }, - verifyFunc: func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) { - // Verify the CNI binary was regenerated - cniBinaryPath := filepath.Join(binDir, consulCNIName) + }, + { + name: "source token chmod event handling", + description: "Test that chmod events on source token trigger re-copy to host", + testFunc: func(t *testing.T, cmd *Command, sourceTokenPath, hostTokenPath string, ctx context.Context) { + // Create initial source token file + initialContent := "chmod-test-token-abc123" + err := os.WriteFile(sourceTokenPath, []byte(initialContent), 0644) + require.NoError(t, err) + + // Start token file watcher + go func() { + err := cmd.tokenFileWatcher(ctx, sourceTokenPath, hostTokenPath) + if err != nil && ctx.Err() == nil { + t.Errorf("tokenFileWatcher error: %v", err) + } + }() + + // Wait for initial copy + time.Sleep(200 * time.Millisecond) + + // Verify initial copy worked retry.Run(t, func(r *retry.R) { - _, err := os.Stat(cniBinaryPath) - require.NoError(r, err, "CNI binary should be regenerated") + content, err := os.ReadFile(hostTokenPath) + require.NoError(r, err, "Host token file should exist") + require.Equal(r, initialContent, string(content), "Initial content should match") }) - // Verify content was copied from source - content, err := os.ReadFile(cniBinaryPath) + + t.Log("Step 1: Update source token content and trigger chmod") + updatedContent := "chmod-updated-token-xyz789" + err = os.WriteFile(sourceTokenPath, []byte(updatedContent), 0600) require.NoError(t, err) - require.Equal(t, "source-binary-content", string(content)) + + // Trigger chmod event which should cause re-copy + err = os.Chmod(sourceTokenPath, 0644) + require.NoError(t, err) + + // Wait for token watcher to detect chmod and copy new content + time.Sleep(300 * time.Millisecond) + + t.Log("Step 2: Verify host token file has updated content after chmod") + retry.Run(t, func(r *retry.R) { + content, err := os.ReadFile(hostTokenPath) + require.NoError(r, err, "Host token file should still exist") + require.Equal(r, updatedContent, string(content), "Host token should have updated content after chmod") + }) }, }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temp directories + tempDir := t.TempDir() + + // Setup paths + sourceTokenPath := filepath.Join(tempDir, "source-token") + hostTokenPath := filepath.Join(tempDir, "host-token") + + // Setup the Command + ui := cli.NewMockUi() + cmd := &Command{ + UI: ui, + } + cmd.init() + var err error + cmd.logger, err = common.Logger("info", false) + require.NoError(t, err) + + // Create context + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + // Run the specific test + tt.testFunc(t, cmd, sourceTokenPath, hostTokenPath, ctx) + }) + } +} + +func TestRun_SignalCleanup(t *testing.T) { + tests := []struct { + name string + signal os.Signal + autorotateToken bool + description string + }{ + { + name: "SIGTERM cleanup with autorotate token", + signal: syscall.SIGTERM, + autorotateToken: true, + description: "Test that SIGTERM signal triggers cleanup of all files including host token", + }, { - name: "kubeconfig file regeneration after removal", + name: "SIGINT cleanup with autorotate token", + signal: os.Interrupt, + autorotateToken: true, + description: "Test that SIGINT signal triggers cleanup of all files including host token", + }, + { + name: "SIGTERM cleanup without autorotate token", + signal: syscall.SIGTERM, autorotateToken: false, - fileToRemove: "ZZZ-consul-cni-kubeconfig", - setupFunc: func(tempDir, binDir string, cfg *config.CNIConfig) (string, error) { - // Create initial kubeconfig file - kubeconfigPath := filepath.Join(tempDir, cfg.Kubeconfig) - err := os.WriteFile(kubeconfigPath, []byte("initial-kubeconfig-content"), 0644) - if err != nil { - return "", err - } - return kubeconfigPath, nil - }, - verifyFunc: func(t *testing.T, tempDir, binDir string, cfg *config.CNIConfig, removedFile string) { - // Verify the kubeconfig file was regenerated - kubeconfigPath := filepath.Join(tempDir, cfg.Kubeconfig) - retry.Run(t, func(r *retry.R) { - _, err := os.Stat(kubeconfigPath) - require.NoError(r, err, "Kubeconfig file should be regenerated") - }) - // Verify file is not empty (createKubeConfig should have generated content) - info, err := os.Stat(kubeconfigPath) - require.NoError(t, err) - require.Greater(t, info.Size(), int64(0), "Kubeconfig should not be empty") - }, + description: "Test that SIGTERM signal triggers cleanup of CNI binary and kubeconfig (no host token)", }, } @@ -204,17 +317,12 @@ func TestRun_FileRegenerationAfterRemoval(t *testing.T) { binDir := t.TempDir() sourceDir := t.TempDir() - // Create source CNI binary for copying - sourceBinaryPath := filepath.Join(sourceDir, consulCNIName) - err := os.WriteFile(sourceBinaryPath, []byte("source-binary-content"), 0755) - require.NoError(t, err) - - // Create a default configuration + // Create configuration uid := fmt.Sprintf("%d", time.Now().UnixNano()) cfg := &config.CNIConfig{ Name: config.DefaultPluginName, - Type: config.DefaultPluginType, - CNITokenPath: filepath.Join(tempDir, "token"), + Type: config.DefaultPluginType + "-" + uid, + CNITokenPath: filepath.Join(sourceDir, "token"), CNIHostTokenPath: func() string { if tt.autorotateToken { return filepath.Join(tempDir, config.DefaultCNIHostTokenFilename+"-"+uid) @@ -229,44 +337,144 @@ func TestRun_FileRegenerationAfterRemoval(t *testing.T) { Multus: false, } - // Setup the Command + // Setup the Command with custom signal channel ui := cli.NewMockUi() cmd := &Command{ UI: ui, flagCNIBinSourceDir: sourceDir, + sigCh: make(chan os.Signal, 1), // Custom signal channel for testing + flagInstallationID: uid, } - cmd.init() + + var err error cmd.logger, err = common.Logger("info", false) require.NoError(t, err) - // Setup initial files - removedFilePath, err := tt.setupFunc(tempDir, binDir, cfg) + // Create all the files and sourceFiles required for test + t.Log("Creating files for test setup...") + + // Create source CNI binary for copying + err = os.WriteFile(filepath.Join(sourceDir, config.DefaultPluginType), []byte("test-binary-content"), 0755) require.NoError(t, err) - // Create context and start directory watcher - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) + // Create a CNI config file to test cleanup + configFile := filepath.Join(cfg.CNINetDir, "10-test.conflist") + configContent := `{ + "cniVersion": "0.3.1", + "name": "test-network", + "plugins": [ + { + "type": "bridge", + "bridge": "cni0" + } + ] + }` + err = os.WriteFile(configFile, []byte(configContent), 0644) + require.NoError(t, err) + + // 2. Kubeconfig file + kubeconfigPath := filepath.Join(cfg.CNINetDir, cfg.Kubeconfig+"-"+uid) + configContentByte, err := os.ReadFile("testdata/ZZZ-consul-cni-kubeconfig-tokenautorotate.golden") + require.NoError(t, err) + err = os.WriteFile(kubeconfigPath, configContentByte, 0644) + require.NoError(t, err) + + // 3. Source token file (if autorotate is enabled) + if tt.autorotateToken { + // Also create source token + err = os.WriteFile(cfg.CNITokenPath, []byte("test-source-token"), 0644) + require.NoError(t, err) + } + + // Start the command in a goroutine + var runResult int + var runErr error + done := make(chan struct{}) go func() { - err := cmd.directoryWatcher(ctx, cfg, tempDir, "") - if err != nil && ctx.Err() == nil { - t.Errorf("directoryWatcher error: %v", err) + defer close(done) + // Create args that would normally be passed to Run + args := []string{ + "-cni-bin-dir", binDir, + "-cni-net-dir", tempDir, + "-bin-source-dir", sourceDir, + "-cni-token-path", sourceDir, + "-kubeconfig", cfg.Kubeconfig, + "-installation-id", uid, + "-log-level", "info", } + if tt.autorotateToken { + args = append(args, "-autorotate-token") + } + + runResult = cmd.Run(args) + }() - // Wait for watcher to initialize - time.Sleep(100 * time.Millisecond) + time.Sleep(2000 * time.Millisecond) + + // Verify all files exist before cleanup + t.Log("Verifying files exist before signal...") + cniBinaryPath := filepath.Join(cfg.CNIBinDir, cfg.Type) + _, err = os.Stat(cniBinaryPath) + require.NoError(t, err, "CNI binary should exist before cleanup") + + // Verify the content of the CNI binary file + content, err := os.ReadFile(cniBinaryPath) + require.NoError(t, err, "Should be able to read CNI binary file") + require.Equal(t, "test-binary-content", string(content), "CNI binary content should match") + + _, err = os.Stat(kubeconfigPath) + require.NoError(t, err, "Kubeconfig should exist before cleanup") + if tt.autorotateToken { + hostTokenPath := filepath.Join(tempDir, config.DefaultCNIHostTokenFilename+"-"+uid) + _, err = os.Stat(hostTokenPath) + require.NoError(t, err, "Host token should exist before cleanup") + + // Verify the content of the host token file + content, err = os.ReadFile(hostTokenPath) + require.NoError(t, err, "Should be able to read host token file") + require.Equal(t, "test-source-token", string(content), "Host token content should match") + } - // Remove the file to trigger regeneration - t.Logf("Removing file: %s", removedFilePath) - err = os.Remove(removedFilePath) - require.NoError(t, err) + t.Logf("Sending %v signal to trigger cleanup...", tt.signal) + // Send the signal to trigger cleanup + cmd.sigCh <- tt.signal + + // Wait for the command to complete + select { + case <-done: + t.Log("Command completed successfully") + case <-time.After(10 * time.Second): + t.Fatal("Command did not complete within timeout") + } - // Wait for file system event to be processed + // Verify the command returned successfully (0) + require.Equal(t, 0, runResult, "Command should return 0 on successful cleanup") + require.NoError(t, runErr, "Command should not return an error") + + // Wait a bit for cleanup to complete time.Sleep(200 * time.Millisecond) - // Verify file regeneration - tt.verifyFunc(t, tempDir, binDir, cfg, removedFilePath) + // Verify all files have been cleaned up + t.Log("Verifying files are cleaned up after signal...") + + // Check CNI binary is removed + _, err = os.Stat(cniBinaryPath) + require.True(t, os.IsNotExist(err), "CNI binary should be removed after cleanup") + + // Check kubeconfig is removed + _, err = os.Stat(kubeconfigPath) + require.True(t, os.IsNotExist(err), "Kubeconfig should be removed after cleanup") + + // Check host token is removed (if it was created) + if tt.autorotateToken { + hostTokenPath := filepath.Join(tempDir, config.DefaultCNIHostTokenFilename+"-"+uid) + _, err = os.Stat(hostTokenPath) + require.True(t, os.IsNotExist(err), "Host token should be removed after cleanup") + } + + t.Log("All files successfully cleaned up after signal") }) } } diff --git a/control-plane/subcommand/install-cni/kubeconfig.go b/control-plane/subcommand/install-cni/kubeconfig.go index 9d30f41824..6ea9b4e265 100644 --- a/control-plane/subcommand/install-cni/kubeconfig.go +++ b/control-plane/subcommand/install-cni/kubeconfig.go @@ -29,7 +29,7 @@ const ( // createKubeConfig creates the kubeconfig file that the consul-cni plugin will use to communicate with the // kubernetes API. -func createKubeConfig(cfg *config.CNIConfig) error { +func createKubeConfig(destDir string, cfg *config.CNIConfig) error { var restCfg *rest.Config // TODO: Move clientset out of this method and put it in 'Run' @@ -70,7 +70,7 @@ func createKubeConfig(cfg *config.CNIConfig) error { } // Write the kubeconfig file to the host. - destFile := filepath.Join(cfg.CNINetDir, cfg.Kubeconfig) + destFile := filepath.Join(destDir, cfg.Kubeconfig) err = os.WriteFile(destFile, data, os.FileMode(0o644)) if err != nil { return fmt.Errorf("error writing kube config file %s: %w", destFile, err)