From 20348d542485327d6248cea00b13ad5133566de8 Mon Sep 17 00:00:00 2001 From: Salah Aldeen Al Saleh Date: Tue, 26 Sep 2023 16:20:32 +0000 Subject: [PATCH] Ensure imagepullsecrets and hooks informer for dynamic namespaces even if API restarts --- pkg/apiserver/server.go | 6 +- pkg/operator/client/client.go | 86 ++++++++++++++----------- pkg/operator/client/client_interface.go | 2 + pkg/operator/client/deploy.go | 4 ++ pkg/operator/client/hooks.go | 2 + pkg/operator/client/namespaces.go | 2 + pkg/operator/helm.go | 44 ------------- pkg/operator/image_pull_secrets.go | 72 +++++++++++++++++++++ pkg/operator/operator.go | 49 ++++++-------- 9 files changed, 152 insertions(+), 115 deletions(-) delete mode 100644 pkg/operator/helm.go create mode 100644 pkg/operator/image_pull_secrets.go diff --git a/pkg/apiserver/server.go b/pkg/apiserver/server.go index 076ebe095d..8938f0986f 100644 --- a/pkg/apiserver/server.go +++ b/pkg/apiserver/server.go @@ -89,9 +89,9 @@ func Start(params *APIServerParams) { if !util.IsHelmManaged() { client := &client.Client{ - TargetNamespace: util.AppNamespace(), - ExistingInformers: map[string]bool{}, - HookStopChans: []chan struct{}{}, + TargetNamespace: util.AppNamespace(), + ExistingHookInformers: map[string]bool{}, + HookStopChans: []chan struct{}{}, } store := store.GetStore() k8sClientset, err := k8sutil.GetClientset() diff --git a/pkg/operator/client/client.go b/pkg/operator/client/client.go index bc6df7693d..652b019b8d 100644 --- a/pkg/operator/client/client.go +++ b/pkg/operator/client/client.go @@ -57,19 +57,18 @@ type Client struct { watchedNamespaces []string imagePullSecrets []string - appStateMonitor *appstate.Monitor - HookStopChans []chan struct{} - namespaceStopChan chan struct{} - ExistingInformers map[string]bool // namespaces map to invoke the Informer once during deploy + appStateMonitor *appstate.Monitor + HookStopChans []chan struct{} + namespaceStopChan chan struct{} + ExistingHookInformers map[string]bool // namespaces map to invoke the Informer once during deploy } func (c *Client) Init() error { - if _, ok := c.ExistingInformers[c.TargetNamespace]; !ok { - c.ExistingInformers[c.TargetNamespace] = true + if _, ok := c.ExistingHookInformers[c.TargetNamespace]; !ok { + c.ExistingHookInformers[c.TargetNamespace] = true if err := c.runHooksInformer(c.TargetNamespace); err != nil { // we don't fail here... - log.Printf("error registering cleanup hooks for TargetNamespace: %s: %s", - c.TargetNamespace, err.Error()) + log.Printf("error registering cleanup hooks for TargetNamespace: %s: %s", c.TargetNamespace, err.Error()) } } @@ -125,6 +124,45 @@ func (c *Client) runAppStateMonitor() error { return errors.New("app state monitor shutdown") } +func (c *Client) RestartNamespacesInformer(namespaces []string, imagePullSecrets []string) { + for _, ns := range namespaces { + if ns == "*" { + continue + } + if err := c.ensureNamespacePresent(ns); err != nil { + // we don't fail here... + log.Printf("error creating namespace: %s", err.Error()) + } + if err := c.ensureImagePullSecretsPresent(ns, imagePullSecrets); err != nil { + // we don't fail here... + log.Printf("error ensuring image pull secrets for namespace %s: %s", ns, err.Error()) + } + } + + c.imagePullSecrets = imagePullSecrets + c.watchedNamespaces = namespaces + + c.shutdownNamespacesInformer() + if len(c.watchedNamespaces) > 0 { + c.runNamespacesInformer() + } +} + +func (c *Client) ApplyHooksInformer(namespaces []string) { + for _, ns := range namespaces { + if ns == "*" { + continue + } + if _, ok := c.ExistingHookInformers[ns]; !ok { + c.ExistingHookInformers[ns] = true + if err := c.runHooksInformer(ns); err != nil { + // we don't fail here... + log.Printf("error registering cleanup hooks for namespace: %s: %s", ns, err.Error()) + } + } + } +} + func (c *Client) DeployApp(deployArgs operatortypes.DeployAppArgs) (deployed bool, finalError error) { log.Println("received a deploy request for", deployArgs.AppSlug) @@ -142,6 +180,9 @@ func (c *Client) DeployApp(deployArgs operatortypes.DeployAppArgs) (deployed boo } }() + c.RestartNamespacesInformer(deployArgs.AdditionalNamespaces, deployArgs.ImagePullSecrets) + c.ApplyHooksInformer(deployArgs.AdditionalNamespaces) + deployRes, deployError = c.deployManifests(deployArgs) if deployError != nil { deployRes = &deployResult{} @@ -160,11 +201,6 @@ func (c *Client) DeployApp(deployArgs operatortypes.DeployAppArgs) (deployed boo return } - c.shutdownNamespacesInformer() - if len(c.watchedNamespaces) > 0 { - c.runNamespacesInformer() - } - return } @@ -214,30 +250,6 @@ func (c *Client) deployManifests(deployArgs operatortypes.DeployAppArgs) (*deplo } } - for _, additionalNamespace := range deployArgs.AdditionalNamespaces { - if additionalNamespace == "*" { - continue - } - if err := c.ensureNamespacePresent(additionalNamespace); err != nil { - // we don't fail here... - log.Printf("error creating namespace: %s", err.Error()) - } - if err := c.ensureImagePullSecretsPresent(additionalNamespace, deployArgs.ImagePullSecrets); err != nil { - // we don't fail here... - log.Printf("error ensuring image pull secrets for namespace %s: %s", additionalNamespace, err.Error()) - } - if _, ok := c.ExistingInformers[additionalNamespace]; !ok { - c.ExistingInformers[additionalNamespace] = true - if err := c.runHooksInformer(additionalNamespace); err != nil { - // we don't fail here... - log.Printf("error registering cleanup hooks for additionalNamespace: %s: %s", - additionalNamespace, err.Error()) - } - } - } - c.imagePullSecrets = deployArgs.ImagePullSecrets - c.watchedNamespaces = deployArgs.AdditionalNamespaces - result, err := c.ensureResourcesPresent(deployArgs) if err != nil { return nil, errors.Wrap(err, "failed to deploy") diff --git a/pkg/operator/client/client_interface.go b/pkg/operator/client/client_interface.go index 975cc95b01..b5d987ca0c 100644 --- a/pkg/operator/client/client_interface.go +++ b/pkg/operator/client/client_interface.go @@ -10,4 +10,6 @@ type ClientInterface interface { DeployApp(deployArgs operatortypes.DeployAppArgs) (deployed bool, finalError error) UndeployApp(undeployArgs operatortypes.UndeployAppArgs) error ApplyAppInformers(args operatortypes.AppInformersArgs) + RestartNamespacesInformer(namespaces []string, imagePullSecrets []string) + ApplyHooksInformer(namespaces []string) } diff --git a/pkg/operator/client/deploy.go b/pkg/operator/client/deploy.go index 47db19d68a..ac43bb28f9 100644 --- a/pkg/operator/client/deploy.go +++ b/pkg/operator/client/deploy.go @@ -44,6 +44,8 @@ type deployResult struct { } func (c *Client) ensureNamespacePresent(name string) error { + logger.Infof("ensuring namespace %s", name) + clientset, err := k8sutil.GetClientset() if err != nil { return errors.Wrap(err, "failed to get clientset") @@ -71,6 +73,8 @@ func (c *Client) ensureNamespacePresent(name string) error { } func (c *Client) ensureImagePullSecretsPresent(namespace string, imagePullSecrets []string) error { + logger.Infof("ensuring image pull secrets for namespace %s", namespace) + imagePullSecretsMtx.Lock() defer imagePullSecretsMtx.Unlock() diff --git a/pkg/operator/client/hooks.go b/pkg/operator/client/hooks.go index fd1019a0d7..fa096c6243 100644 --- a/pkg/operator/client/hooks.go +++ b/pkg/operator/client/hooks.go @@ -16,6 +16,8 @@ import ( // runHooksInformer will create goroutines to start various informers for kots objects func (c *Client) runHooksInformer(namespace string) error { + logger.Infof("running hooks informer for namespace %s", namespace) + clientset, err := k8sutil.GetClientset() if err != nil { return errors.Wrap(err, "failed to get clientset") diff --git a/pkg/operator/client/namespaces.go b/pkg/operator/client/namespaces.go index b84c22565f..7e6f16394c 100644 --- a/pkg/operator/client/namespaces.go +++ b/pkg/operator/client/namespaces.go @@ -44,6 +44,8 @@ func (c *Client) runNamespacesInformer() error { // we don't fail here... log.Printf("error ensuring image pull secrets for namespace %s: %s", addedNamespace.Name, err.Error()) } + + c.ApplyHooksInformer([]string{addedNamespace.Name}) } }, }) diff --git a/pkg/operator/helm.go b/pkg/operator/helm.go deleted file mode 100644 index 28e686efc1..0000000000 --- a/pkg/operator/helm.go +++ /dev/null @@ -1,44 +0,0 @@ -package operator - -import ( - "io/ioutil" - "os" - "path/filepath" - - "github.com/pkg/errors" - "github.com/replicatedhq/kots/pkg/util" -) - -func getChartsImagePullSecrets(deployedVersionArchive string) ([]string, error) { - archiveChartDir := filepath.Join(deployedVersionArchive, "overlays", "midstream", "charts") - chartDirs, err := ioutil.ReadDir(archiveChartDir) - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - return nil, errors.Wrap(err, "failed to read charts directory") - } - - imagePullSecrets := []string{} - for _, chartDir := range chartDirs { - if !chartDir.IsDir() { - continue - } - - secretFilename := filepath.Join(archiveChartDir, chartDir.Name(), "secret.yaml") - secretData, err := ioutil.ReadFile(secretFilename) - if err != nil { - if os.IsNotExist(err) { - continue - } - return nil, errors.Wrap(err, "failed to read helm tar.gz file") - } - - secrets := util.ConvertToSingleDocs(secretData) - for _, secret := range secrets { - imagePullSecrets = append(imagePullSecrets, string(secret)) - } - } - - return imagePullSecrets, nil -} diff --git a/pkg/operator/image_pull_secrets.go b/pkg/operator/image_pull_secrets.go new file mode 100644 index 0000000000..2b71d13f40 --- /dev/null +++ b/pkg/operator/image_pull_secrets.go @@ -0,0 +1,72 @@ +package operator + +import ( + "os" + "path/filepath" + + "github.com/pkg/errors" + "github.com/replicatedhq/kots/pkg/util" +) + +func getImagePullSecrets(deployedVersionArchive string) ([]string, error) { + imagePullSecrets := []string{} + + secretFilename := filepath.Join(deployedVersionArchive, "overlays", "midstream", "secret.yaml") + _, err := os.Stat(secretFilename) + if err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "failed to os stat image pull secret file") + } + if err == nil { + b, err := os.ReadFile(secretFilename) + if err != nil { + return nil, errors.Wrap(err, "failed to read image pull secret file") + } + secrets := util.ConvertToSingleDocs(b) + for _, secret := range secrets { + imagePullSecrets = append(imagePullSecrets, string(secret)) + } + } + + chartPullSecrets, err := getChartsImagePullSecrets(deployedVersionArchive) + if err != nil { + return nil, errors.Wrap(err, "failed to read image pull secret files from charts") + } + imagePullSecrets = append(imagePullSecrets, chartPullSecrets...) + imagePullSecrets = deduplicateSecrets(imagePullSecrets) + + return imagePullSecrets, nil +} + +func getChartsImagePullSecrets(deployedVersionArchive string) ([]string, error) { + archiveChartDir := filepath.Join(deployedVersionArchive, "overlays", "midstream", "charts") + chartDirs, err := os.ReadDir(archiveChartDir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, errors.Wrap(err, "failed to read charts directory") + } + + imagePullSecrets := []string{} + for _, chartDir := range chartDirs { + if !chartDir.IsDir() { + continue + } + + secretFilename := filepath.Join(archiveChartDir, chartDir.Name(), "secret.yaml") + secretData, err := os.ReadFile(secretFilename) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, errors.Wrap(err, "failed to read helm tar.gz file") + } + + secrets := util.ConvertToSingleDocs(secretData) + for _, secret := range secrets { + imagePullSecrets = append(imagePullSecrets, string(secret)) + } + } + + return imagePullSecrets, nil +} diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index c2217cabc8..dd5dbc13e4 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -6,7 +6,6 @@ import ( "encoding/base64" "encoding/json" "fmt" - "io/ioutil" "os" "path/filepath" "strconv" @@ -92,7 +91,7 @@ func (o *Operator) Start() error { } o.clusterID = id - go o.resumeStatusInformers() + go o.resumeInformers() go o.resumeDeployments() startLoop(o.restoreLoop, 2) @@ -216,7 +215,7 @@ func (o *Operator) DeployApp(appID string, sequence int64) (deployed bool, deplo return false, errors.Wrap(err, "failed to get downstream") } - deployedVersionArchive, err := ioutil.TempDir("", "kotsadm") + deployedVersionArchive, err := os.MkdirTemp("", "kotsadm") if err != nil { return false, errors.Wrap(err, "failed to create temp dir") } @@ -324,30 +323,10 @@ func (o *Operator) DeployApp(appID string, sequence int64) (deployed bool, deplo return false, errors.Wrap(err, "failed to get v1beta2 charts archive") } - imagePullSecrets := []string{} - secretFilename := filepath.Join(deployedVersionArchive, "overlays", "midstream", "secret.yaml") - _, err = os.Stat(secretFilename) - if err != nil && !os.IsNotExist(err) { - return false, errors.Wrap(err, "failed to os stat image pull secret file") - } - if err == nil { - b, err := ioutil.ReadFile(secretFilename) - if err != nil { - return false, errors.Wrap(err, "failed to read image pull secret file") - } - secrets := util.ConvertToSingleDocs(b) - for _, secret := range secrets { - imagePullSecrets = append(imagePullSecrets, string(secret)) - } - } - - chartPullSecrets, err := getChartsImagePullSecrets(deployedVersionArchive) + imagePullSecrets, err := getImagePullSecrets(deployedVersionArchive) if err != nil { - deployError = errors.Wrap(err, "failed to read image pull secret files from charts") - return false, deployError + return false, errors.Wrap(err, "failed to get image pull secrets") } - imagePullSecrets = append(imagePullSecrets, chartPullSecrets...) - imagePullSecrets = deduplicateSecrets(imagePullSecrets) // get previous manifests (if any) var previousKotsKinds *kotsutil.KotsKinds @@ -365,7 +344,7 @@ func (o *Operator) DeployApp(appID string, sequence int64) (deployed bool, deplo } if previouslyDeployedParentSequence != -1 { - previouslyDeployedVersionArchive, err := ioutil.TempDir("", "kotsadm") + previouslyDeployedVersionArchive, err := os.MkdirTemp("", "kotsadm") if err != nil { return false, errors.Wrap(err, "failed to create temp dir") } @@ -489,20 +468,20 @@ func (o *Operator) applyStatusInformers(a *apptypes.App, sequence int64, kotsKin return nil } -func (o *Operator) resumeStatusInformers() { +func (o *Operator) resumeInformers() { apps, err := o.store.ListAppsForDownstream(o.clusterID) if err != nil { logger.Error(errors.Wrap(err, "failed to list installed apps for downstream")) return } for _, app := range apps { - if err := o.resumeStatusInformersForApp(app); err != nil { + if err := o.resumeInformersForApp(app); err != nil { logger.Error(errors.Wrapf(err, "failed to resume status informers for app %s in cluster %s", app.ID, o.clusterID)) } } } -func (o *Operator) resumeStatusInformersForApp(app *apptypes.App) error { +func (o *Operator) resumeInformersForApp(app *apptypes.App) error { deployedVersion, err := o.store.GetCurrentDownstreamVersion(app.ID, o.clusterID) if err != nil { return errors.Wrap(err, "failed to get current downstream version") @@ -513,7 +492,7 @@ func (o *Operator) resumeStatusInformersForApp(app *apptypes.App) error { logger.Debugf("starting status informers for app %s", app.ID) - deployedVersionArchive, err := ioutil.TempDir("", "kotsadm") + deployedVersionArchive, err := os.MkdirTemp("", "kotsadm") if err != nil { return errors.Wrap(err, "failed to create temp dir") } @@ -524,6 +503,11 @@ func (o *Operator) resumeStatusInformersForApp(app *apptypes.App) error { return errors.Wrap(err, "failed to get app version archive") } + imagePullSecrets, err := getImagePullSecrets(deployedVersionArchive) + if err != nil { + return errors.Wrap(err, "failed to get image pull secrets") + } + kotsKinds, err := kotsutil.LoadKotsKindsFromPath(filepath.Join(deployedVersionArchive, "upstream")) if err != nil { return errors.Wrap(err, "failed to load kotskinds") @@ -543,6 +527,9 @@ func (o *Operator) resumeStatusInformersForApp(app *apptypes.App) error { return errors.Wrapf(err, "failed to apply status informers for app %s", app.ID) } + o.client.RestartNamespacesInformer(kotsKinds.KotsApplication.Spec.AdditionalNamespaces, imagePullSecrets) + o.client.ApplyHooksInformer(kotsKinds.KotsApplication.Spec.AdditionalNamespaces) + return nil } @@ -727,7 +714,7 @@ func (o *Operator) UndeployApp(a *apptypes.App, d *downstreamtypes.Downstream, i return nil } - deployedVersionArchive, err := ioutil.TempDir("", "kotsadm") + deployedVersionArchive, err := os.MkdirTemp("", "kotsadm") if err != nil { return errors.Wrap(err, "failed to create temp dir") }