diff --git a/.golangci.yml b/.golangci.yml index b5fa706d..0c2df182 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -48,6 +48,7 @@ linters: - wsl # too many empty lines makes methods too long - wrapcheck # forces to wrap errors everywhere - lll # Just useless in the most cases + - perfsprint # to keep errors consistent issues: diff --git a/cli-tests/playwright.config.ts b/cli-tests/playwright.config.ts index d01a77c8..53b87fc0 100644 --- a/cli-tests/playwright.config.ts +++ b/cli-tests/playwright.config.ts @@ -31,7 +31,7 @@ export default defineConfig({ forbidOnly: !!process.env.CI, /* Retry on CI only */ retries: process.env.CI ? 2 : 0, - timeout: 300_000, + timeout: 600_000, /* Opt out of parallel tests on CI. */ workers: process.env.CI ? 1 : undefined, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ diff --git a/cli-tests/tests/flow/all-operators.spec.ts b/cli-tests/tests/flow/all-operators.spec.ts index b0cc5de8..2a0a93fd 100644 --- a/cli-tests/tests/flow/all-operators.spec.ts +++ b/cli-tests/tests/flow/all-operators.spec.ts @@ -98,11 +98,14 @@ test.describe('Everest CLI install', async () => { ); await out.assertSuccess(); - // check that the deployment does not exist - out = await cli.exec('kubectl get deploy percona-everest -n everest-system'); + // check that the namespace does not exist + out = await cli.exec('kubectl get ns everest-system everest-monitoring everest-olm everest-all'); await out.outErrContainsNormalizedMany([ - 'Error from server (NotFound): deployments.apps "percona-everest" not found', + 'Error from server (NotFound): namespaces "everest-system" not found', + 'Error from server (NotFound): namespaces "everest-monitoring" not found', + 'Error from server (NotFound): namespaces "everest-olm" not found', + 'Error from server (NotFound): namespaces "everest-all" not found', ]); }); diff --git a/commands/install.go b/commands/install.go index 38edb793..4b57c412 100644 --- a/commands/install.go +++ b/commands/install.go @@ -29,7 +29,13 @@ import ( func newInstallCmd(l *zap.SugaredLogger) *cobra.Command { cmd := &cobra.Command{ - Use: "install", + Use: "install", + // The command expects no arguments. So to prevent users from misspelling and confusion + // in cases with unexpected spaces like + // ./everestctl install --namespaces=aaa, a + // it will return + // Error: unknown command "a" for "everestctl install" + Args: cobra.NoArgs, Example: "everestctl install --namespaces dev,staging,prod --operator.mongodb=true --operator.postgresql=true --operator.xtradb-cluster=true --skip-wizard", Run: func(cmd *cobra.Command, args []string) { initInstallViperFlags(cmd) diff --git a/commands/upgrade.go b/commands/upgrade.go index bbc4472a..8341f783 100644 --- a/commands/upgrade.go +++ b/commands/upgrade.go @@ -31,6 +31,12 @@ import ( func newUpgradeCmd(l *zap.SugaredLogger) *cobra.Command { cmd := &cobra.Command{ Use: "upgrade", + // The command expects no arguments. So to prevent users from misspelling and confusion + // in cases with unexpected spaces like + // ./everestctl upgrade --namespaces=aaa, a + // it will return + // Error: unknown command "a" for "everestctl upgrade" + Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { initUpgradeViperFlags(cmd) diff --git a/go.mod b/go.mod index 201988af..3e4614b0 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/hashicorp/go-version v1.6.0 github.com/operator-framework/api v0.22.0 github.com/operator-framework/operator-lifecycle-manager v0.26.0 - github.com/percona/everest-operator v0.6.0-dev1.0.20240214112044-8f2dea595284 + github.com/percona/everest-operator v0.6.0-dev1.0.20240220114053-fae6111d9818 github.com/percona/percona-everest-backend v0.7.0 github.com/spf13/cobra v1.8.0 github.com/spf13/viper v1.18.2 diff --git a/go.sum b/go.sum index 57fdef1d..87a71546 100644 --- a/go.sum +++ b/go.sum @@ -536,8 +536,8 @@ github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/9 github.com/pelletier/go-toml v1.7.0/go.mod h1:vwGMzjaWMwyfHwgIBhI2YUM4fB6nL6lVAvS1LBMMhTE= github.com/pelletier/go-toml/v2 v2.1.0 h1:FnwAJ4oYMvbT/34k9zzHuZNrhlz48GB3/s6at6/MHO4= github.com/pelletier/go-toml/v2 v2.1.0/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc= -github.com/percona/everest-operator v0.6.0-dev1.0.20240214112044-8f2dea595284 h1:5LKWEvGtaimDPVI3Rj9tFxO+g/zAjUJq0yFIzznQGfc= -github.com/percona/everest-operator v0.6.0-dev1.0.20240214112044-8f2dea595284/go.mod h1:45pGpvWrPy495qiQqxNuOJor4wif+vTTTJP4Qee8qZk= +github.com/percona/everest-operator v0.6.0-dev1.0.20240220114053-fae6111d9818 h1:w4E4zlSTRQQk2/tFAFO5WGquvKRg2ocw7hxcbRjUT58= +github.com/percona/everest-operator v0.6.0-dev1.0.20240220114053-fae6111d9818/go.mod h1:45pGpvWrPy495qiQqxNuOJor4wif+vTTTJP4Qee8qZk= github.com/percona/percona-backup-mongodb v1.8.1-0.20230920143330-3b1c2e263901 h1:BDgsZRCjEuxl2/z4yWBqB0s8d20shuIDks7/RVdZiLs= github.com/percona/percona-backup-mongodb v1.8.1-0.20230920143330-3b1c2e263901/go.mod h1:fZRCMpUqkWlLVdRKqqaj001LoVP2eo6F0ZhoMPeXDng= github.com/percona/percona-everest-backend v0.7.0 h1:ku03G3p1sttWL9rfzeOhUcSarREnszmeWScTqGMynzk= diff --git a/install.sh b/install.sh index cab215c4..d3d473cb 100755 --- a/install.sh +++ b/install.sh @@ -32,8 +32,6 @@ else echo "KUBECONFIG is not set. Using default k8s cluster" fi -echo "Provisioning Everest with monitoring disabled" -echo "If you want to enable monitoring please refer to the everest installation documentation." echo "" ./everestctl install --namespaces everest --operator.mongodb=true --operator.postgresql=true --operator.xtradb-cluster=true --skip-wizard diff --git a/pkg/install/install.go b/pkg/install/install.go index 45c3cfcc..60612db7 100644 --- a/pkg/install/install.go +++ b/pkg/install/install.go @@ -23,6 +23,7 @@ import ( "fmt" "net/url" "os" + "regexp" "strings" "github.com/AlecAivazis/survey/v2" @@ -88,11 +89,29 @@ const ( disableTelemetryEnvVar = "DISABLE_TELEMETRY" ) +//nolint:gochecknoglobals +var ( + // ErrNSEmpty appears when the provided list of the namespaces is considered empty. + ErrNSEmpty = errors.New("namespace list is empty. Specify at least one namespace") + // ErrNSReserved appears when some of the provided names are forbidden to use. + ErrNSReserved = func(ns string) error { + return fmt.Errorf("'%s' namespace is reserved for Everest internals. Please specify another namespace", ns) + } + // ErrNameNotRFC1035Compatible appears when some of the provided names are not RFC1035 compatible. + ErrNameNotRFC1035Compatible = func(fieldName string) error { + return fmt.Errorf(`'%s' is not RFC 1035 compatible. The name should contain only lowercase alphanumeric characters or '-', start with an alphabetic character, end with an alphanumeric character`, + fieldName, + ) + } +) + type ( // Config stores configuration for the operators. Config struct { - // Namespaces defines comma-separated list of namespaces that everest can operate in. + // Namespaces is a user-defined string represents raw non-validated comma-separated list of namespaces for everest to operate in. Namespaces string `mapstructure:"namespaces"` + // NamespacesList validated list of namespaces that everest can operate in. + NamespacesList []string `mapstructure:"namespaces-map"` // SkipWizard skips wizard during installation. SkipWizard bool `mapstructure:"skip-wizard"` // KubeconfigPath is a path to a kubeconfig @@ -114,11 +133,6 @@ type ( } ) -// NamespacesList returns list of the namespaces that everest can operate in. -func (c Config) NamespacesList() []string { - return strings.Split(c.Namespaces, ",") -} - // NewInstall returns a new Install struct. func NewInstall(c Config, l *zap.SugaredLogger) (*Install, error) { cli := &Install{ @@ -205,15 +219,11 @@ func (o *Install) populateConfig() error { } } - if len(o.config.NamespacesList()) == 0 { - return errors.New("namespace list is empty. Specify the comma-separated list of namespaces using the --namespaces flag, at least one namespace is required") - } - - for _, ns := range o.config.NamespacesList() { - if ns == SystemNamespace || ns == MonitoringNamespace { - return fmt.Errorf("'%s' namespace is reserved for Everest internals. Please specify another namespace", ns) - } + l, err := ValidateNamespaces(o.config.Namespaces) + if err != nil { + return err } + o.config.NamespacesList = l return nil } @@ -295,7 +305,7 @@ func (o *Install) provisionEverestOperator(ctx context.Context, recVer *version. } o.l.Info("Creating operator group for everest") - if err := o.kubeClient.CreateOperatorGroup(ctx, systemOperatorGroup, SystemNamespace, o.config.NamespacesList()); err != nil { + if err := o.kubeClient.CreateOperatorGroup(ctx, systemOperatorGroup, SystemNamespace, o.config.NamespacesList); err != nil { return err } @@ -337,7 +347,7 @@ func (o *Install) provisionEverest(ctx context.Context, v *goversion.Version) er } o.l.Info("Updating cluster role bindings for everest-admin") - if err := o.kubeClient.UpdateClusterRoleBinding(ctx, everestServiceAccountClusterRoleBinding, o.config.NamespacesList()); err != nil { + if err := o.kubeClient.UpdateClusterRoleBinding(ctx, everestServiceAccountClusterRoleBinding, o.config.NamespacesList); err != nil { return err } @@ -345,7 +355,7 @@ func (o *Install) provisionEverest(ctx context.Context, v *goversion.Version) er } func (o *Install) provisionDBNamespaces(ctx context.Context, recVer *version.RecommendedVersion) error { - for _, namespace := range o.config.NamespacesList() { + for _, namespace := range o.config.NamespacesList { namespace := namespace if err := o.createNamespace(namespace); err != nil { return err @@ -399,27 +409,12 @@ func (o *Install) runEverestWizard() error { return err } - nsList := strings.Split(namespaces, ",") - for _, ns := range nsList { - ns = strings.TrimSpace(ns) - if ns == "" { - continue - } - - if ns == SystemNamespace { - return fmt.Errorf("'%s' namespace is reserved for Everest internals. Please specify another namespace", ns) - } - - if o.config.Namespaces != "" { - o.config.Namespaces += "," - } - - o.config.Namespaces += ns - } - - if len(o.config.NamespacesList()) == 0 { - return errors.New("namespace list is empty. Specify at least one namespace") + list, err := ValidateNamespaces(namespaces) + if err != nil { + return err } + o.config.Namespaces = namespaces + o.config.NamespacesList = list return nil } @@ -577,7 +572,7 @@ func (o *Install) installOperator(ctx context.Context, channel, operatorName, na }, } if operatorName == everestOperatorName { - params.TargetNamespaces = o.config.NamespacesList() + params.TargetNamespaces = o.config.NamespacesList params.SubscriptionConfig.Env = append(params.SubscriptionConfig.Env, []corev1.EnvVar{ { Name: EverestMonitoringNamespaceEnvVar, @@ -585,7 +580,7 @@ func (o *Install) installOperator(ctx context.Context, channel, operatorName, na }, { Name: kubernetes.EverestDBNamespacesEnvVar, - Value: o.config.Namespaces, + Value: strings.Join(o.config.NamespacesList, ","), }, }...) } @@ -671,3 +666,46 @@ func (o *Install) generateToken(ctx context.Context) (*token.ResetResponse, erro return res, nil } + +// ValidateNamespaces validates a comma-separated namespaces string. +func ValidateNamespaces(str string) ([]string, error) { + nsList := strings.Split(str, ",") + m := make(map[string]struct{}) + for _, ns := range nsList { + ns = strings.TrimSpace(ns) + if ns == "" { + continue + } + + if ns == SystemNamespace || ns == MonitoringNamespace { + return nil, ErrNSReserved(ns) + } + + if err := validateRFC1035(ns); err != nil { + return nil, err + } + + m[ns] = struct{}{} + } + + list := make([]string, 0, len(m)) + for k := range m { + list = append(list, k) + } + + if len(list) == 0 { + return nil, ErrNSEmpty + } + return list, nil +} + +// validates names to be RFC-1035 compatible https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names +func validateRFC1035(s string) error { + rfc1035Regex := "^[a-z]([-a-z0-9]{0,61}[a-z0-9])?$" + re := regexp.MustCompile(rfc1035Regex) + if !re.MatchString(s) { + return ErrNameNotRFC1035Compatible(s) + } + + return nil +} diff --git a/pkg/install/install_test.go b/pkg/install/install_test.go new file mode 100644 index 00000000..7598dff1 --- /dev/null +++ b/pkg/install/install_test.go @@ -0,0 +1,98 @@ +package install + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateNamespaces(t *testing.T) { + t.Parallel() + + type tcase struct { + name string + input string + output []string + error error + } + + tcases := []tcase{ + { + name: "empty string", + input: "", + output: nil, + error: ErrNSEmpty, + }, + { + name: "several empty strings", + input: " , ,", + output: nil, + error: ErrNSEmpty, + }, + { + name: "correct", + input: "aaa,bbb,ccc", + output: []string{"aaa", "bbb", "ccc"}, + error: nil, + }, + { + name: "correct with spaces", + input: ` aaa, bbb +,ccc `, + output: []string{"aaa", "bbb", "ccc"}, + error: nil, + }, + { + name: "reserved system ns", + input: "everest-system", + output: nil, + error: ErrNSReserved("everest-system"), + }, + { + name: "reserved system ns and empty ns", + input: "everest-system, ", + output: nil, + error: ErrNSReserved("everest-system"), + }, + { + name: "reserved monitoring ns", + input: "everest-monitoring", + output: nil, + error: ErrNSReserved("everest-monitoring"), + }, + { + name: "duplicated ns", + input: "aaa,bbb,aaa", + output: []string{"aaa", "bbb"}, + error: nil, + }, + { + name: "name is too long", + input: "e1234567890123456789012345678901234567890123456789012345678901234567890,bbb", + output: nil, + error: ErrNameNotRFC1035Compatible("e1234567890123456789012345678901234567890123456789012345678901234567890"), + }, + { + name: "name starts with number", + input: "1aaa,bbb", + output: nil, + error: ErrNameNotRFC1035Compatible("1aaa"), + }, + { + name: "name contains special characters", + input: "aa12a,b$s", + output: nil, + error: ErrNameNotRFC1035Compatible("b$s"), + }, + } + + for _, tc := range tcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + output, err := ValidateNamespaces(tc.input) + assert.Equal(t, tc.error, err) + assert.ElementsMatch(t, tc.output, output) + }) + } +} diff --git a/pkg/kubernetes/backup_storage.go b/pkg/kubernetes/backup_storage.go index 93a01b5f..b490e258 100644 --- a/pkg/kubernetes/backup_storage.go +++ b/pkg/kubernetes/backup_storage.go @@ -79,21 +79,21 @@ func (k *Kubernetes) IsBackupStorageUsed(ctx context.Context, namespace, backupS if err != nil { return false, err } - if len(list.Items) > 0 { + if len(list.Items) != 0 { return true, nil } bList, err := k.client.ListDatabaseClusterBackups(ctx, namespace, options) if err != nil { return false, err } - if len(bList.Items) > 0 { + if len(bList.Items) != 0 { return true, nil } rList, err := k.client.ListDatabaseClusterRestores(ctx, namespace, options) if err != nil { return false, err } - if len(rList.Items) > 0 { + if len(rList.Items) != 0 { return true, nil } } diff --git a/pkg/kubernetes/client/client.go b/pkg/kubernetes/client/client.go index e140d659..84c2e73a 100644 --- a/pkg/kubernetes/client/client.go +++ b/pkg/kubernetes/client/client.go @@ -71,7 +71,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "github.com/percona/percona-everest-cli/pkg/kubernetes/client/customresources" - "github.com/percona/percona-everest-cli/pkg/kubernetes/client/database" ) const ( @@ -104,7 +103,6 @@ type Client struct { customClientSet *customresources.Client apiextClientset apiextv1clientset.Interface dynamicClientset dynamic.Interface - dbClusterClient *database.DBClusterClient rcLock *sync.Mutex restConfig *rest.Config namespace string diff --git a/pkg/uninstall/uninstall.go b/pkg/uninstall/uninstall.go index 6f68a2b9..11f47bb5 100644 --- a/pkg/uninstall/uninstall.go +++ b/pkg/uninstall/uninstall.go @@ -174,7 +174,7 @@ func (u *Uninstall) getDBs(ctx context.Context) (map[string]*everestv1alpha1.Dat return nil, err } - allDBs := map[string]*everestv1alpha1.DatabaseClusterList{} + allDBs := make(map[string]*everestv1alpha1.DatabaseClusterList) for _, ns := range namespaces { dbs, err := u.kubeClient.ListDatabaseClusters(ctx, ns) if err != nil { @@ -226,6 +226,20 @@ func (u *Uninstall) deleteDBs(ctx context.Context) error { // Wait for all database clusters to be deleted, or timeout after 5 minutes. u.l.Info("Waiting for database clusters to be deleted") + // XXX: When deleting a DBC CR, the everest operator doesn't wait for the + // DB operator's CRs to be deleted. Thus, as soon as we delete the DBC CRs, + // these cease to exist in the cluster and the polling below will return + // immediately. If we don't wait for the DB operators to process the + // deletion of the CRs, we may end up deleting the namespaces before the DB + // operators have a chance to delete the resources they manage, leaving the + // namespaces in an endless Terminating state waiting for finalizers to be + // removed. + // The everest operator should have a Deleting status that waits for the DB + // operators to delete their DB CRs before removing the corresponting DBC + // CR. Until this is implemented, we work around this by sleeping for two + // minutes to give the DB operators a chance to delete the resources they + // manage before we delete the namespaces. + time.Sleep(2 * time.Minute) return wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, false, func(ctx context.Context) (bool, error) { allDBs, err := u.getDBs(ctx) if err != nil {