From 47cd827076f44324d3453973ef3af476fcb5b187 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Wed, 10 Jul 2024 21:07:17 +0400 Subject: [PATCH 01/19] Ignore .vscode folder --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a2f58b2f0..3c2d828af 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ /parts/ /stage/ /prime/ +.vscode/ **.snap From 9ce4f98916d54c5211c296977c7e7b17817e642c Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Mon, 22 Jul 2024 13:39:47 +0400 Subject: [PATCH 02/19] Add api type, Add k8sd type, Add migration and queries, Add db methods --- src/k8s/api/v1/types.go | 21 +++++ src/k8s/go.mod | 3 +- src/k8s/pkg/k8sd/database/feature_status.go | 79 +++++++++++++++++++ .../pkg/k8sd/database/feature_status_test.go | 11 +++ src/k8s/pkg/k8sd/database/schema.go | 1 + .../feature-status/000-feature-status.sql | 9 +++ .../sql/queries/feature-status/select.sql | 4 + .../sql/queries/feature-status/upsert.sql | 9 +++ src/k8s/pkg/k8sd/types/feature_status.go | 32 ++++++++ src/k8s/pkg/k8sd/types/feature_status_test.go | 44 +++++++++++ 10 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 src/k8s/pkg/k8sd/database/feature_status.go create mode 100644 src/k8s/pkg/k8sd/database/feature_status_test.go create mode 100644 src/k8s/pkg/k8sd/database/sql/migrations/feature-status/000-feature-status.sql create mode 100644 src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql create mode 100644 src/k8s/pkg/k8sd/database/sql/queries/feature-status/upsert.sql create mode 100644 src/k8s/pkg/k8sd/types/feature_status.go create mode 100644 src/k8s/pkg/k8sd/types/feature_status_test.go diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index e3a737e40..808b6c172 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -3,6 +3,7 @@ package apiv1 import ( "fmt" "strings" + "time" "gopkg.in/yaml.v2" ) @@ -42,6 +43,18 @@ type NodeStatus struct { DatastoreRole DatastoreRole `json:"datastore-role,omitempty"` } +// FeatureStatus encapsulates the deployment status of a feature. +type FeatureStatus struct { + // Enabled shows whether or not the deployment of manifests for a status was successful. + Enabled bool + // Message contains information about the status of a feature. It is only supposed to be human readable and informative and should not be programmatically parsed. + Message string + // Version shows the version of the deployed feature. + Version string + // Timestamp shows when the last update was done. + Timestamp time.Time +} + type Datastore struct { Type string `json:"type,omitempty"` Servers []string `json:"servers,omitempty" yaml:"servers,omitempty"` @@ -54,6 +67,14 @@ type ClusterStatus struct { Members []NodeStatus `json:"members,omitempty"` Config UserFacingClusterConfig `json:"config,omitempty"` Datastore Datastore `json:"datastore,omitempty"` + + DNS FeatureStatus `json:"dns,omitempty"` + Network FeatureStatus `json:"network,omitempty"` + LoadBalancer FeatureStatus `json:"load-balancer,omitempty"` + Ingress FeatureStatus `json:"ingress,omitempty"` + Gateway FeatureStatus `json:"gateway,omitempty"` + MetricsServer FeatureStatus `json:"metrics-server,omitempty"` + LocalStorage FeatureStatus `json:"local-storage,omitempty"` } // HaClusterFormed returns true if the cluster is in high-availability mode (more than two voter nodes). diff --git a/src/k8s/go.mod b/src/k8s/go.mod index d6387051a..efd3bd57b 100644 --- a/src/k8s/go.mod +++ b/src/k8s/go.mod @@ -12,6 +12,7 @@ require ( github.com/onsi/gomega v1.32.0 github.com/pelletier/go-toml v1.9.5 github.com/spf13/cobra v1.8.0 + github.com/stretchr/testify v1.9.0 golang.org/x/net v0.23.0 golang.org/x/sys v0.19.0 gopkg.in/yaml.v2 v2.4.0 @@ -122,6 +123,7 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/pkg/sftp v1.13.6 // indirect github.com/pkg/xattr v0.4.9 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.19.0 // indirect github.com/prometheus/client_model v0.6.0 // indirect github.com/prometheus/common v0.51.1 // indirect @@ -133,7 +135,6 @@ require ( github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/cast v1.6.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/stretchr/objx v0.5.2 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xeipuuv/gojsonschema v1.2.0 // indirect diff --git a/src/k8s/pkg/k8sd/database/feature_status.go b/src/k8s/pkg/k8sd/database/feature_status.go new file mode 100644 index 000000000..90fb6218b --- /dev/null +++ b/src/k8s/pkg/k8sd/database/feature_status.go @@ -0,0 +1,79 @@ +package database + +import ( + "context" + "database/sql" + "fmt" + "time" + + "github.com/canonical/k8s/pkg/k8sd/types" + "github.com/canonical/microcluster/cluster" +) + +var featureStatusStmts = struct { + select_ int + upsert_ int +}{ + select_: MustPrepareStatement("feature-status", "select.sql"), + upsert_: MustPrepareStatement("feature-status", "upsert.sql"), +} + +// SetFeatureStatus updates the status of the given feature. +func SetFeatureStatus(ctx context.Context, tx *sql.Tx, name string, status types.FeatureStatus) error { + upsertTxStmt, err := cluster.Stmt(tx, featureStatusStmts.upsert_) + if err != nil { + return fmt.Errorf("failed to prepare upsert statement: %w", err) + } + + if _, err := upsertTxStmt.ExecContext(ctx, + name, + status.Message, + status.Version, + status.Timestamp.Format(time.RFC3339), + status.Enabled, + ); err != nil { + return fmt.Errorf("failed to execute upsert statement: %w", err) + } + + return nil +} + +// GetFeatureStatuses returns a map of feature names to their status. +func GetFeatureStatuses(ctx context.Context, tx *sql.Tx) (map[string]types.FeatureStatus, error) { + selectTxStmt, err := cluster.Stmt(tx, featureStatusStmts.select_) + if err != nil { + return nil, fmt.Errorf("failed to prepare select statement: %w", err) + } + + rows, err := selectTxStmt.QueryContext(ctx) + if err != nil { + return nil, fmt.Errorf("failed to execute select statement: %w", err) + } + + fsMap := make(map[string]types.FeatureStatus) + + for rows.Next() { + var ( + name string + ts string + ) + typ := types.FeatureStatus{} + + if err := rows.Scan(&name, &typ.Message, &typ.Version, &ts, &typ.Enabled); err != nil { + return nil, fmt.Errorf("failed to scan row: %w", err) + } + + typ.Timestamp, err = time.Parse(time.RFC3339, ts) + if err != nil { + return nil, fmt.Errorf("failed to parse timestamp: %w", err) + } + + fsMap[name] = typ + } + + if rows.Err() != nil { + return nil, fmt.Errorf("failed to read rows: %w", err) + } + + return fsMap, nil +} diff --git a/src/k8s/pkg/k8sd/database/feature_status_test.go b/src/k8s/pkg/k8sd/database/feature_status_test.go new file mode 100644 index 000000000..5303536a1 --- /dev/null +++ b/src/k8s/pkg/k8sd/database/feature_status_test.go @@ -0,0 +1,11 @@ +package database_test + +import "testing" + +func TestSetFeatureStatus(t *testing.T) { + t.FailNow() +} + +func TestGetFeatureStatuses(t *testing.T) { + t.FailNow() +} diff --git a/src/k8s/pkg/k8sd/database/schema.go b/src/k8s/pkg/k8sd/database/schema.go index 2e88fb545..ec7c8669a 100644 --- a/src/k8s/pkg/k8sd/database/schema.go +++ b/src/k8s/pkg/k8sd/database/schema.go @@ -16,6 +16,7 @@ var ( schemaApplyMigration("kubernetes-auth-tokens", "000-create.sql"), schemaApplyMigration("cluster-configs", "000-create.sql"), schemaApplyMigration("worker-tokens", "000-create.sql"), + schemaApplyMigration("feature-status", "000-feature-status.sql"), } //go:embed sql/migrations diff --git a/src/k8s/pkg/k8sd/database/sql/migrations/feature-status/000-feature-status.sql b/src/k8s/pkg/k8sd/database/sql/migrations/feature-status/000-feature-status.sql new file mode 100644 index 000000000..a61bb1e99 --- /dev/null +++ b/src/k8s/pkg/k8sd/database/sql/migrations/feature-status/000-feature-status.sql @@ -0,0 +1,9 @@ +CREATE TABLE feature_status ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + name TEXT UNIQUE NOT NULL, + message TEXT NOT NULL, + version TEXT NOT NULL, + timestamp TEXT NOT NULL, + enabled BOOLEAN NOT NULL, + UNIQUE(name) +) diff --git a/src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql b/src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql new file mode 100644 index 000000000..d1549ec42 --- /dev/null +++ b/src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql @@ -0,0 +1,4 @@ +SELECT + name, message, version, timestamp, enabled +FROM + feature_status \ No newline at end of file diff --git a/src/k8s/pkg/k8sd/database/sql/queries/feature-status/upsert.sql b/src/k8s/pkg/k8sd/database/sql/queries/feature-status/upsert.sql new file mode 100644 index 000000000..ddcfcc829 --- /dev/null +++ b/src/k8s/pkg/k8sd/database/sql/queries/feature-status/upsert.sql @@ -0,0 +1,9 @@ +INSERT INTO + feature_status(name, message, version, timestamp, enabled) +VALUES + (?, ?, ?, ?, ?) +ON CONFLICT(name) DO UPDATE SET + message=excluded.message, + version=excluded.version, + timestamp=excluded.timestamp, + enabled=excluded.enabled; diff --git a/src/k8s/pkg/k8sd/types/feature_status.go b/src/k8s/pkg/k8sd/types/feature_status.go new file mode 100644 index 000000000..7d886d39b --- /dev/null +++ b/src/k8s/pkg/k8sd/types/feature_status.go @@ -0,0 +1,32 @@ +package types + +import ( + "time" + + apiv1 "github.com/canonical/k8s/api/v1" +) + +type FeatureStatus struct { + Enabled bool + Message string + Version string + Timestamp time.Time +} + +func (f FeatureStatus) ToAPI() (apiv1.FeatureStatus, error) { + return apiv1.FeatureStatus{ + Enabled: f.Enabled, + Message: f.Message, + Version: f.Version, + Timestamp: f.Timestamp, + }, nil +} + +func FeatureStatusFromAPI(apiFS apiv1.FeatureStatus) (FeatureStatus, error) { + return FeatureStatus{ + Enabled: apiFS.Enabled, + Message: apiFS.Message, + Version: apiFS.Version, + Timestamp: apiFS.Timestamp, + }, nil +} diff --git a/src/k8s/pkg/k8sd/types/feature_status_test.go b/src/k8s/pkg/k8sd/types/feature_status_test.go new file mode 100644 index 000000000..623062e06 --- /dev/null +++ b/src/k8s/pkg/k8sd/types/feature_status_test.go @@ -0,0 +1,44 @@ +package types_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + apiv1 "github.com/canonical/k8s/api/v1" + "github.com/canonical/k8s/pkg/k8sd/types" +) + +func TestK8sdFeatureStatusToAPI(t *testing.T) { + k8sdFS := types.FeatureStatus{ + Enabled: true, + Message: "message", + Version: "version", + Timestamp: time.Now(), + } + + apiFS, err := k8sdFS.ToAPI() + require.NoError(t, err) + assert.Equal(t, k8sdFS.Enabled, apiFS.Enabled) + assert.Equal(t, k8sdFS.Message, apiFS.Message) + assert.Equal(t, k8sdFS.Version, apiFS.Version) + assert.Equal(t, k8sdFS.Timestamp, apiFS.Timestamp) +} + +func TestAPIFeatureStatusToK8sd(t *testing.T) { + apiFS := apiv1.FeatureStatus{ + Enabled: true, + Message: "message", + Version: "version", + Timestamp: time.Now(), + } + + k8sdFS, err := types.FeatureStatusFromAPI(apiFS) + require.NoError(t, err) + assert.Equal(t, apiFS.Enabled, k8sdFS.Enabled) + assert.Equal(t, apiFS.Message, k8sdFS.Message) + assert.Equal(t, apiFS.Version, k8sdFS.Version) + assert.Equal(t, apiFS.Timestamp, k8sdFS.Timestamp) +} From e70f2c383db37682184b5248a2c5212fc40c4d9d Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 09:57:10 +0400 Subject: [PATCH 03/19] Update getClusterStatus, Update controller, Update all features --- src/k8s/pkg/k8sd/api/cluster.go | 31 ++++++++ src/k8s/pkg/k8sd/app/hooks_start.go | 16 ++++ src/k8s/pkg/k8sd/controllers/feature.go | 77 +++++++++++++------ src/k8s/pkg/k8sd/features/calico/network.go | 46 ++++++++--- src/k8s/pkg/k8sd/features/cilium/gateway.go | 71 ++++++++++++++--- src/k8s/pkg/k8sd/features/cilium/ingress.go | 54 +++++++++++-- .../pkg/k8sd/features/cilium/loadbalancer.go | 32 ++++++-- src/k8s/pkg/k8sd/features/cilium/network.go | 56 +++++++++++--- src/k8s/pkg/k8sd/features/cilium/status.go | 5 ++ src/k8s/pkg/k8sd/features/contour/gateway.go | 41 ++++++++-- src/k8s/pkg/k8sd/features/contour/ingress.go | 54 ++++++++++--- src/k8s/pkg/k8sd/features/coredns/coredns.go | 39 ++++++++-- src/k8s/pkg/k8sd/features/interface.go | 42 +++++----- src/k8s/pkg/k8sd/features/localpv/localpv.go | 40 +++++++++- .../pkg/k8sd/features/metallb/loadbalancer.go | 34 ++++++-- .../features/metrics-server/metrics_server.go | 39 +++++++++- .../metrics-server/metrics_server_test.go | 10 ++- 17 files changed, 558 insertions(+), 129 deletions(-) diff --git a/src/k8s/pkg/k8sd/api/cluster.go b/src/k8s/pkg/k8sd/api/cluster.go index 5f16accfa..9430e6c72 100644 --- a/src/k8s/pkg/k8sd/api/cluster.go +++ b/src/k8s/pkg/k8sd/api/cluster.go @@ -1,11 +1,14 @@ package api import ( + "context" + "database/sql" "fmt" "net/http" apiv1 "github.com/canonical/k8s/api/v1" "github.com/canonical/k8s/pkg/k8sd/api/impl" + "github.com/canonical/k8s/pkg/k8sd/database" databaseutil "github.com/canonical/k8s/pkg/k8sd/database/util" "github.com/canonical/lxd/lxd/response" "github.com/canonical/microcluster/state" @@ -36,6 +39,27 @@ func (e *Endpoints) getClusterStatus(s *state.State, r *http.Request) response.R return response.InternalError(fmt.Errorf("failed to check if cluster has ready nodes: %w", err)) } + featureStatuses := make(map[string]apiv1.FeatureStatus) + if err := s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { + statuses, err := database.GetFeatureStatuses(s.Context, tx) + if err != nil { + return fmt.Errorf("failed to get feature statuses: %w", err) + } + + for name, st := range statuses { + apiSt, err := st.ToAPI() + if err != nil { + return fmt.Errorf("failed to convert k8sd feature status to api feature status: %w", err) + } + + featureStatuses[name] = apiSt + } + + return nil + }); err != nil { + return response.InternalError(fmt.Errorf("database transaction failed: %w", err)) + } + result := apiv1.GetClusterStatusResponse{ ClusterStatus: apiv1.ClusterStatus{ Ready: ready, @@ -45,6 +69,13 @@ func (e *Endpoints) getClusterStatus(s *state.State, r *http.Request) response.R Type: config.Datastore.GetType(), Servers: config.Datastore.GetExternalServers(), }, + DNS: featureStatuses["dns"], + Network: featureStatuses["network"], + LoadBalancer: featureStatuses["load-balancer"], + Ingress: featureStatuses["ingress"], + Gateway: featureStatuses["gateway"], + MetricsServer: featureStatuses["metrics-server"], + LocalStorage: featureStatuses["local-storage"], }, } diff --git a/src/k8s/pkg/k8sd/app/hooks_start.go b/src/k8s/pkg/k8sd/app/hooks_start.go index cd606268d..c4416a207 100644 --- a/src/k8s/pkg/k8sd/app/hooks_start.go +++ b/src/k8s/pkg/k8sd/app/hooks_start.go @@ -5,6 +5,7 @@ import ( "crypto/rsa" "database/sql" "fmt" + "time" "github.com/canonical/k8s/pkg/k8sd/database" databaseutil "github.com/canonical/k8s/pkg/k8sd/database/util" @@ -72,6 +73,21 @@ func (a *App) onStart(s *state.State) error { return nil }, + func(ctx context.Context, name string, featureStatus types.FeatureStatus) error { + if err := s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { + // we set timestamp here in order to reduce the clutter. otherwise we will need to + // set .Timestamp field in a lot of places for every event/error. + // this is not 100% accurate but should be good enough + featureStatus.Timestamp = time.Now() + if err := database.SetFeatureStatus(ctx, tx, name, featureStatus); err != nil { + return fmt.Errorf("failed to set feature status in db for '%s': %w", name, err) + } + return nil + }); err != nil { + return fmt.Errorf("database transaction to set feature status failed: %w", err) + } + return nil + }, ) } diff --git a/src/k8s/pkg/k8sd/controllers/feature.go b/src/k8s/pkg/k8sd/controllers/feature.go index 13ea26666..86397994d 100644 --- a/src/k8s/pkg/k8sd/controllers/feature.go +++ b/src/k8s/pkg/k8sd/controllers/feature.go @@ -69,65 +69,96 @@ func NewFeatureController(opts FeatureControllerOpts) *FeatureController { } } -func (c *FeatureController) Run(ctx context.Context, getClusterConfig func(context.Context) (types.ClusterConfig, error), notifyDNSChangedIP func(ctx context.Context, dnsIP string) error) { +func (c *FeatureController) Run( + ctx context.Context, + getClusterConfig func(context.Context) (types.ClusterConfig, error), + notifyDNSChangedIP func(ctx context.Context, dnsIP string) error, + setFeatureStatus func(ctx context.Context, name string, featureStatus types.FeatureStatus) error, +) { c.waitReady() ctx = log.NewContext(ctx, log.FromContext(ctx).WithValues("controller", "feature")) - go c.reconcileLoop(ctx, getClusterConfig, "network", c.triggerNetworkCh, c.reconciledNetworkCh, func(cfg types.ClusterConfig) error { + go c.reconcileLoop(ctx, getClusterConfig, "network", c.triggerNetworkCh, c.reconciledNetworkCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyNetwork(ctx, c.snap, cfg.Network, cfg.Annotations) - }) + }, setFeatureStatus) - go c.reconcileLoop(ctx, getClusterConfig, "gateway", c.triggerGatewayCh, c.reconciledGatewayCh, func(cfg types.ClusterConfig) error { + go c.reconcileLoop(ctx, getClusterConfig, "gateway", c.triggerGatewayCh, c.reconciledGatewayCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyGateway(ctx, c.snap, cfg.Gateway, cfg.Network, cfg.Annotations) - }) + }, setFeatureStatus) - go c.reconcileLoop(ctx, getClusterConfig, "ingress", c.triggerIngressCh, c.reconciledIngressCh, func(cfg types.ClusterConfig) error { + go c.reconcileLoop(ctx, getClusterConfig, "ingress", c.triggerIngressCh, c.reconciledIngressCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyIngress(ctx, c.snap, cfg.Ingress, cfg.Network, cfg.Annotations) - }) + }, setFeatureStatus) - go c.reconcileLoop(ctx, getClusterConfig, "load balancer", c.triggerLoadBalancerCh, c.reconciledLoadBalancerCh, func(cfg types.ClusterConfig) error { + go c.reconcileLoop(ctx, getClusterConfig, "load-balancer", c.triggerLoadBalancerCh, c.reconciledLoadBalancerCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyLoadBalancer(ctx, c.snap, cfg.LoadBalancer, cfg.Network, cfg.Annotations) - }) + }, setFeatureStatus) - go c.reconcileLoop(ctx, getClusterConfig, "local storage", c.triggerLocalStorageCh, c.reconciledLocalStorageCh, func(cfg types.ClusterConfig) error { + go c.reconcileLoop(ctx, getClusterConfig, "local-storage", c.triggerLocalStorageCh, c.reconciledLocalStorageCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyLocalStorage(ctx, c.snap, cfg.LocalStorage, cfg.Annotations) - }) + }, setFeatureStatus) - go c.reconcileLoop(ctx, getClusterConfig, "metrics server", c.triggerMetricsServerCh, c.reconciledMetricsServerCh, func(cfg types.ClusterConfig) error { + go c.reconcileLoop(ctx, getClusterConfig, "metrics-server", c.triggerMetricsServerCh, c.reconciledMetricsServerCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyMetricsServer(ctx, c.snap, cfg.MetricsServer, cfg.Annotations) - }) + }, setFeatureStatus) - go c.reconcileLoop(ctx, getClusterConfig, "DNS", c.triggerDNSCh, c.reconciledDNSCh, func(cfg types.ClusterConfig) error { - if dnsIP, err := features.Implementation.ApplyDNS(ctx, c.snap, cfg.DNS, cfg.Kubelet, cfg.Annotations); err != nil { - return fmt.Errorf("failed to apply DNS configuration: %w", err) + go c.reconcileLoop(ctx, getClusterConfig, "dns", c.triggerDNSCh, c.reconciledDNSCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + featureStatus, dnsIP, err := features.Implementation.ApplyDNS(ctx, c.snap, cfg.DNS, cfg.Kubelet, cfg.Annotations) + + if err != nil { + return featureStatus, fmt.Errorf("failed to apply DNS configuration: %w", err) } else if dnsIP != "" { if err := notifyDNSChangedIP(ctx, dnsIP); err != nil { - return fmt.Errorf("failed to update DNS IP address to %s: %w", dnsIP, err) + // we already have featureStatus.Message which contains wrapped error of the Apply + // (or empty if no error occurs). we further wrap the error to add the DNS IP change error to the message + changeErr := fmt.Errorf("failed to update DNS IP address to %s: %w", dnsIP, err) + featureStatus.Message = fmt.Sprintf("%s: %v", featureStatus.Message, changeErr) + return featureStatus, changeErr } } - return nil - }) + return featureStatus, nil + }, setFeatureStatus) } -func (c *FeatureController) reconcile(ctx context.Context, getClusterConfig func(context.Context) (types.ClusterConfig, error), apply func(cfg types.ClusterConfig) error) error { +func (c *FeatureController) reconcile( + ctx context.Context, + getClusterConfig func(context.Context) (types.ClusterConfig, error), + apply func(cfg types.ClusterConfig) (types.FeatureStatus, error), + componentName string, + setFeatureStatus func(context.Context, string, types.FeatureStatus) error, +) error { cfg, err := getClusterConfig(ctx) if err != nil { return fmt.Errorf("failed to retrieve cluster configuration: %w", err) } - if err := apply(cfg); err != nil { + featureStatus, err := apply(cfg) + if err != nil { return fmt.Errorf("failed to apply configuration: %w", err) } + + if err := setFeatureStatus(ctx, componentName, featureStatus); err != nil { + return fmt.Errorf("failed to set feature status for '%s': %w", err) + } + return nil } -func (c *FeatureController) reconcileLoop(ctx context.Context, getClusterConfig func(context.Context) (types.ClusterConfig, error), componentName string, triggerCh chan struct{}, reconciledCh chan<- struct{}, apply func(cfg types.ClusterConfig) error) { +func (c *FeatureController) reconcileLoop( + ctx context.Context, + getClusterConfig func(context.Context) (types.ClusterConfig, error), + componentName string, + triggerCh chan struct{}, + reconciledCh chan<- struct{}, + apply func(cfg types.ClusterConfig) (types.FeatureStatus, error), + setFeatureStatus func(ctx context.Context, name string, status types.FeatureStatus) error, +) { for { select { case <-ctx.Done(): return case <-triggerCh: - if err := c.reconcile(ctx, getClusterConfig, apply); err != nil { + if err := c.reconcile(ctx, getClusterConfig, apply, componentName, setFeatureStatus); err != nil { log.FromContext(ctx).WithValues("feature", componentName).Error(err, "Failed to apply feature configuration") // notify triggerCh after 5 seconds to retry diff --git a/src/k8s/pkg/k8sd/features/calico/network.go b/src/k8s/pkg/k8sd/features/calico/network.go index 756b6d842..0657711e3 100644 --- a/src/k8s/pkg/k8sd/features/calico/network.go +++ b/src/k8s/pkg/k8sd/features/calico/network.go @@ -10,28 +10,51 @@ import ( "github.com/canonical/k8s/pkg/utils" ) +const ( + enabledMsg = "enabled" + disabledMsg = "disabled" + deployFailedMsgTmpl = "Failed to deploy Calico, the error was: %v" + deleteFailedMsgTmpl = "Failed to delete Calico, the error was: %v" +) + // ApplyNetwork will deploy Calico when cfg.Enabled is true. // ApplyNetwork will remove Calico when cfg.Enabled is false. -// ApplyNetwork returns an error if anything fails. -func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annotations types.Annotations) error { +// ApplyNetwork will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyNetwork returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annotations types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: calicoTag, + Enabled: cfg.GetEnabled(), + } + m := snap.HelmClient() if !cfg.GetEnabled() { if _, err := m.Apply(ctx, chartCalico, helm.StateDeleted, nil); err != nil { - return fmt.Errorf("failed to uninstall network: %w", err) + applyErr := fmt.Errorf("failed to uninstall network: %w", err) + status.Message = fmt.Sprintf(deleteFailedMsgTmpl, applyErr) + return status, applyErr } - return nil + status.Version = "" + status.Message = disabledMsg + return status, nil } config, err := internalConfig(annotations) if err != nil { - return fmt.Errorf("failed to parse annotations: %w", err) + cfgErr := fmt.Errorf("failed to parse annotations: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, cfgErr) + return status, cfgErr } podIpPools := []map[string]any{} ipv4PodCIDR, ipv6PodCIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR()) if err != nil { - return fmt.Errorf("invalid pod cidr: %v", err) + cidrErr := fmt.Errorf("invalid pod cidr: %v", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, cidrErr) + return status, cidrErr } if ipv4PodCIDR != "" { podIpPools = append(podIpPools, map[string]any{ @@ -51,7 +74,9 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota serviceCIDRs := []string{} ipv4ServiceCIDR, ipv6ServiceCIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR()) if err != nil { - return fmt.Errorf("invalid service cidr: %v", err) + cidrErr := fmt.Errorf("invalid service cidr: %v", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, cidrErr) + return status, cidrErr } if ipv4ServiceCIDR != "" { serviceCIDRs = append(serviceCIDRs, ipv4ServiceCIDR) @@ -93,8 +118,11 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota } if _, err := m.Apply(ctx, chartCalico, helm.StatePresent, values); err != nil { - return fmt.Errorf("failed to enable network: %w", err) + enableErr := fmt.Errorf("failed to enable network: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + return status, enableErr } - return nil + status.Message = enabledMsg + return status, nil } diff --git a/src/k8s/pkg/k8sd/features/cilium/gateway.go b/src/k8s/pkg/k8sd/features/cilium/gateway.go index 7fe41c682..a263165c9 100644 --- a/src/k8s/pkg/k8sd/features/cilium/gateway.go +++ b/src/k8s/pkg/k8sd/features/cilium/gateway.go @@ -9,33 +9,86 @@ import ( "github.com/canonical/k8s/pkg/snap" ) +const ( + gatewayDeleteFailedMsgTmpl = "Failed to delete Cilium Gateway, the error was %v" + gatewayDeployFailedMsgTmpl = "Failed to deploy Cilium Gateway, the error was %v" +) + // ApplyGateway assumes that the managed Cilium CNI is already installed on the cluster. It will fail if that is not the case. // ApplyGateway will deploy the Gateway API CRDs on the cluster and enable the GatewayAPI controllers on Cilium, when gateway.Enabled is true. // ApplyGateway will remove the Gateway API CRDs from the cluster and disable the GatewayAPI controllers on Cilium, when gateway.Enabled is false. // ApplyGateway will rollout restart the Cilium pods in case any Cilium configuration was changed. -// ApplyGateway returns an error if anything fails. -func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, _ types.Annotations) error { +// ApplyGateway will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyGateway returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: ciliumAgentImageTag, + Enabled: gateway.GetEnabled(), + } m := snap.HelmClient() if _, err := m.Apply(ctx, chartGateway, helm.StatePresentOrDeleted(gateway.GetEnabled()), nil); err != nil { - return fmt.Errorf("failed to install Gateway API CRDs: %w", err) + if gateway.GetEnabled() { + enableErr := fmt.Errorf("failed to install Gateway API CRDs: %w", err) + status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) + return status, enableErr + } else { + disableErr := fmt.Errorf("failed to delete Gateway API CRDs: %w", err) + status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, disableErr) + return status, disableErr + } } // Apply our GatewayClass named ck-gateway if _, err := m.Apply(ctx, chartGatewayClass, helm.StatePresentOrDeleted(gateway.GetEnabled()), nil); err != nil { - return fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) + if gateway.GetEnabled() { + enableErr := fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) + status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) + return status, enableErr + } else { + disableErr := fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) + status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, disableErr) + return status, disableErr + } } changed, err := m.Apply(ctx, chartCilium, helm.StateUpgradeOnlyOrDeleted(network.GetEnabled()), map[string]any{"gatewayAPI": map[string]any{"enabled": gateway.GetEnabled()}}) if err != nil { - return fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) + if gateway.GetEnabled() { + enableErr := fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) + status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) + return status, enableErr + } else { + disableErr := fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) + status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, disableErr) + return status, disableErr + } } - if !changed || !gateway.GetEnabled() { - return nil + if !changed { + if gateway.GetEnabled() { + status.Message = enabledMsg + return status, nil + } else { + status.Message = disabledMsg + status.Version = "" + return status, nil + } + } + + if !gateway.GetEnabled() { + status.Message = disabledMsg + status.Version = "" + return status, nil } if err := rolloutRestartCilium(ctx, snap, 3); err != nil { - return fmt.Errorf("failed to rollout restart cilium to apply Gateway API: %w", err) + resErr := fmt.Errorf("failed to rollout restart cilium to apply Gateway API: %w", err) + status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, resErr) + return status, resErr } - return nil + + status.Message = enabledMsg + return status, nil } diff --git a/src/k8s/pkg/k8sd/features/cilium/ingress.go b/src/k8s/pkg/k8sd/features/cilium/ingress.go index 56fd4bcf8..49b1d1aab 100644 --- a/src/k8s/pkg/k8sd/features/cilium/ingress.go +++ b/src/k8s/pkg/k8sd/features/cilium/ingress.go @@ -9,12 +9,24 @@ import ( "github.com/canonical/k8s/pkg/snap" ) +const ( + ingressDeleteFailedMsgTmpl = "Failed to delete Cilium Ingress, the error was: %v" + ingressDeployFailedMsgTmpl = "Failed to deploy Cilium Ingress, the error was: %v" +) + // ApplyIngress assumes that the managed Cilium CNI is already installed on the cluster. It will fail if that is not the case. // ApplyIngress will enable Cilium's ingress controller when ingress.Enabled is true. -// ApplyIngress will disable Cilium's ingress controller when ingress.Disabled is false. +// ApplyIngress will disable Cilium's ingress controller when ingress.Enabled is false. // ApplyIngress will rollout restart the Cilium pods in case any Cilium configuration was changed. -// ApplyIngress returns an error if anything fails. -func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, network types.Network, _ types.Annotations) error { +// ApplyIngress will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyIngress returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: ciliumAgentImageTag, + Enabled: ingress.GetEnabled(), + } m := snap.HelmClient() var values map[string]any @@ -41,14 +53,40 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, ne changed, err := m.Apply(ctx, chartCilium, helm.StateUpgradeOnlyOrDeleted(network.GetEnabled()), values) if err != nil { - return fmt.Errorf("failed to enable ingress: %w", err) + if network.GetEnabled() { + enableErr := fmt.Errorf("failed to enable ingress: %w", err) + status.Message = fmt.Sprint(ingressDeployFailedMsgTmpl, enableErr) + return status, enableErr + } else { + disableErr := fmt.Errorf("failed to disable ingress: %w", err) + status.Message = fmt.Sprint(ingressDeleteFailedMsgTmpl, disableErr) + return status, disableErr + } } - if !changed || !ingress.GetEnabled() { - return nil + + if !changed { + if ingress.GetEnabled() { + status.Message = enabledMsg + return status, nil + } else { + status.Message = disabledMsg + status.Version = "" + return status, nil + } + } + + if !ingress.GetEnabled() { + status.Message = disabledMsg + status.Version = "" + return status, nil } if err := rolloutRestartCilium(ctx, snap, 3); err != nil { - return fmt.Errorf("failed to rollout restart cilium to apply ingress: %w", err) + restartErr := fmt.Errorf("failed to rollout restart cilium to apply ingress: %w", err) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, restartErr) + return status, restartErr } - return nil + + status.Message = enabledMsg + return status, nil } diff --git a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go index 5ea8549fb..7ce5b4a3e 100644 --- a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go @@ -10,23 +10,43 @@ import ( "github.com/canonical/k8s/pkg/utils/control" ) +const ( + lbDeleteFailedMsgTmpl = "Failed to delete Cilium Load Balancer, the error was: %v" + lbDeployFailedMsgTmpl = "Failed to deploy Cilium Load Balancer, the error was: %v" +) + // ApplyLoadBalancer assumes that the managed Cilium CNI is already installed on the cluster. It will fail if that is not the case. // ApplyLoadBalancer will configure Cilium to enable L2 or BGP mode, and deploy necessary CRs for announcing the LoadBalancer external IPs when loadbalancer.Enabled is true. // ApplyLoadBalancer will disable L2 and BGP on Cilium, and remove any previously created CRs when loadbalancer.Enabled is false. // ApplyLoadBalancer will rollout restart the Cilium pods in case any Cilium configuration was changed. -// ApplyLoadBalancer returns an error if anything fails. -func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, _ types.Annotations) error { +// ApplyLoadBalancer will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyLoadBalancer returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: ciliumAgentImageTag, + Enabled: loadbalancer.GetEnabled(), + } + if !loadbalancer.GetEnabled() { if err := disableLoadBalancer(ctx, snap, network); err != nil { - return fmt.Errorf("failed to disable LoadBalancer: %w", err) + disErr := fmt.Errorf("failed to disable LoadBalancer: %w", err) + status.Message = fmt.Sprintf(lbDeleteFailedMsgTmpl, disErr) + return status, disErr } - return nil + status.Message = disabledMsg + status.Version = "" + return status, nil } if err := enableLoadBalancer(ctx, snap, loadbalancer, network); err != nil { - return fmt.Errorf("failed to enable LoadBalancer: %w", err) + enableErr := fmt.Errorf("failed to enable LoadBalancer: %w", err) + status.Message = fmt.Sprintf(lbDeployFailedMsgTmpl, enableErr) + return status, enableErr } - return nil + status.Message = enabledMsg + return status, nil } func disableLoadBalancer(ctx context.Context, snap snap.Snap, network types.Network) error { diff --git a/src/k8s/pkg/k8sd/features/cilium/network.go b/src/k8s/pkg/k8sd/features/cilium/network.go index ade80715d..857ca25a4 100644 --- a/src/k8s/pkg/k8sd/features/cilium/network.go +++ b/src/k8s/pkg/k8sd/features/cilium/network.go @@ -12,24 +12,42 @@ import ( "github.com/canonical/k8s/pkg/utils/control" ) +const ( + networkDeleteFailedMsgTmpl = "Failed to delete Cilium Network, the error was: %v" + networkDeployFailedMsgTmpl = "Failed to deploy Cilium Network, the error was: %v" +) + // ApplyNetwork will deploy Cilium when cfg.Enabled is true. // ApplyNetwork will remove Cilium when cfg.Enabled is false. // ApplyNetwork requires that bpf and cgroups2 are already mounted and available when running under strict snap confinement. If they are not, it will fail (since Cilium will not have the required permissions to mount them). // ApplyNetwork requires that `/sys` is mounted as a shared mount when running under classic snap confinement. This is to ensure that Cilium will be able to automatically mount bpf and cgroups2 on the pods. -// ApplyNetwork returns an error if anything fails. -func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ types.Annotations) error { +// ApplyNetwork will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyNetwork returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: ciliumAgentImageTag, + Enabled: cfg.GetEnabled(), + } m := snap.HelmClient() if !cfg.GetEnabled() { if _, err := m.Apply(ctx, chartCilium, helm.StateDeleted, nil); err != nil { - return fmt.Errorf("failed to uninstall network: %w", err) + uninstallErr := fmt.Errorf("failed to uninstall network: %w", err) + status.Message = fmt.Sprintf(networkDeleteFailedMsgTmpl, uninstallErr) + return status, uninstallErr } - return nil + status.Message = disabledMsg + status.Version = "" + return status, nil } ipv4CIDR, ipv6CIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR()) if err != nil { - return fmt.Errorf("invalid kube-proxy --cluster-cidr value: %v", err) + cidrErr := fmt.Errorf("invalid kube-proxy --cluster-cidr value: %v", err) + status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, cidrErr) + return status, cidrErr } values := map[string]any{ @@ -74,12 +92,16 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if snap.Strict() { bpfMnt, err := utils.GetMountPath("bpf") if err != nil { - return fmt.Errorf("failed to get bpf mount path: %w", err) + mntErr := fmt.Errorf("failed to get bpf mount path: %w", err) + status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, mntErr) + return status, mntErr } cgrMnt, err := utils.GetMountPath("cgroup2") if err != nil { - return fmt.Errorf("failed to get cgroup2 mount path: %w", err) + cgrpErr := fmt.Errorf("failed to get cgroup2 mount path: %w", err) + status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, cgrpErr) + return status, cgrpErr } values["bpf"] = map[string]any{ @@ -97,7 +119,9 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type } else { p, err := utils.GetMountPropagation("/sys") if err != nil { - return fmt.Errorf("failed to get mount propagation for %s: %w", p, err) + mntErr := fmt.Errorf("failed to get mount propagation for %s: %w", p, err) + status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, mntErr) + return status, mntErr } if p == "private" { onLXD, err := snap.OnLXD(ctx) @@ -105,17 +129,25 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type log.FromContext(ctx).Error(err, "Failed to check if running on LXD") } if onLXD { - return fmt.Errorf("/sys is not a shared mount on the LXD container, this might be resolved by updating LXD on the host to version 5.0.2 or newer") + lxdErr := fmt.Errorf("/sys is not a shared mount on the LXD container, this might be resolved by updating LXD on the host to version 5.0.2 or newer") + status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, lxdErr) + return status, lxdErr } - return fmt.Errorf("/sys is not a shared mount") + + sysErr := fmt.Errorf("/sys is not a shared mount") + status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, sysErr) + return status, sysErr } } if _, err := m.Apply(ctx, chartCilium, helm.StatePresent, values); err != nil { - return fmt.Errorf("failed to enable network: %w", err) + enableErr := fmt.Errorf("failed to enable network: %w", err) + status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, enableErr) + return status, enableErr } - return nil + status.Message = enabledMsg + return status, nil } func rolloutRestartCilium(ctx context.Context, snap snap.Snap, attempts int) error { diff --git a/src/k8s/pkg/k8sd/features/cilium/status.go b/src/k8s/pkg/k8sd/features/cilium/status.go index 45b61c79b..ba9fc4739 100644 --- a/src/k8s/pkg/k8sd/features/cilium/status.go +++ b/src/k8s/pkg/k8sd/features/cilium/status.go @@ -9,6 +9,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + disabledMsg = "disabled" + enabledMsg = "enabled" +) + func CheckNetwork(ctx context.Context, snap snap.Snap) error { client, err := snap.KubernetesClient("kube-system") if err != nil { diff --git a/src/k8s/pkg/k8sd/features/contour/gateway.go b/src/k8s/pkg/k8sd/features/contour/gateway.go index 10149170a..df86de1da 100644 --- a/src/k8s/pkg/k8sd/features/contour/gateway.go +++ b/src/k8s/pkg/k8sd/features/contour/gateway.go @@ -10,27 +10,49 @@ import ( "github.com/canonical/k8s/pkg/utils/control" ) +const ( + enabledMsg = "enabled" + disabledMsg = "disabled" + gatewayDeployFailedMsgTmpl = "Failed to deploy Contour Gateway, the error was: %v" + gatewayDeleteFailedMsgTmpl = "Failed to delete Contour Gateway, the error was: %v" +) + // ApplyGateway will install a helm chart for contour-gateway-provisioner on the cluster when gateway.Enabled is true. // ApplyGateway will uninstall the helm chart for contour-gateway-provisioner from the cluster when gateway.Enabled is false. -// ApplyGateway returns an error if anything fails. // ApplyGateway will apply common contour CRDS, these are shared with ingress. -func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, _ types.Annotations) error { +// ApplyGateway will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyGateway returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: contourGatewayProvisionerContourImageTag, + Enabled: gateway.GetEnabled(), + } m := snap.HelmClient() if !gateway.GetEnabled() { if _, err := m.Apply(ctx, chartGateway, helm.StateDeleted, nil); err != nil { - return fmt.Errorf("failed to uninstall the contour gateway chart: %w", err) + delErr := fmt.Errorf("failed to uninstall the contour gateway chart: %w", err) + status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, delErr) + return status, delErr } - return nil + status.Message = disabledMsg + status.Version = "" + return status, nil } // Apply common contour CRDS, these are shared with ingress if err := applyCommonContourCRDS(ctx, snap, true); err != nil { - return fmt.Errorf("failed to apply common contour CRDS: %w", err) + crdErr := fmt.Errorf("failed to apply common contour CRDS: %w", err) + status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, crdErr) + return status, crdErr } if err := waitForRequiredContourCommonCRDs(ctx, snap); err != nil { - return fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) + waitErr := fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) + status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, waitErr) + return status, waitErr } values := map[string]any{ @@ -49,10 +71,13 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne } if _, err := m.Apply(ctx, chartGateway, helm.StatePresent, values); err != nil { - return fmt.Errorf("failed to install the contour gateway chart: %w", err) + installErr := fmt.Errorf("failed to install the contour gateway chart: %w", err) + status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, installErr) + return status, installErr } - return nil + status.Message = enabledMsg + return status, nil } // waitForRequiredContourCommonCRDs waits for the required contour CRDs to be available diff --git a/src/k8s/pkg/k8sd/features/contour/ingress.go b/src/k8s/pkg/k8sd/features/contour/ingress.go index 9a1a08785..d9b43c3d1 100644 --- a/src/k8s/pkg/k8sd/features/contour/ingress.go +++ b/src/k8s/pkg/k8sd/features/contour/ingress.go @@ -10,30 +10,50 @@ import ( "github.com/canonical/k8s/pkg/utils/control" ) +const ( + ingressDeleteFailedMsgTmpl = "Failed to delete Contour Ingress, the error was: %v" + ingressDeployFailedMsgTmpl = "Failed to deploy Contour Ingress, the error was: %v" +) + // ApplyIngress will install the contour helm chart when ingress.Enabled is true. // ApplyIngress will uninstall the contour helm chart when ingress.Disabled is false. // ApplyIngress will rollout restart the Contour pods in case any Contour configuration was changed. // ApplyIngress will install a delegation resource via helm chart // for the default TLS secret if ingress.DefaultTLSSecret is set. -// ApplyIngress returns an error if anything fails. +// ApplyIngress will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyIngress returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. // Contour CRDS are applied through a ck-contour common chart (Overlap with gateway) -func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ types.Network, _ types.Annotations) error { +func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ types.Network, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: contourIngressContourImageTag, + Enabled: ingress.GetEnabled(), + } m := snap.HelmClient() if !ingress.GetEnabled() { if _, err := m.Apply(ctx, chartContour, helm.StateDeleted, nil); err != nil { - return fmt.Errorf("failed to uninstall ingress: %w", err) + delErr := fmt.Errorf("failed to uninstall ingress: %w", err) + status.Message = fmt.Sprintf(ingressDeleteFailedMsgTmpl, delErr) + return status, delErr } - return nil + status.Message = disabledMsg + status.Version = "" + return status, nil } // Apply common contour CRDS, these are shared with gateway if err := applyCommonContourCRDS(ctx, snap, true); err != nil { - return fmt.Errorf("failed to apply common contour CRDS: %w", err) + crdErr := fmt.Errorf("failed to apply common contour CRDS: %w", err) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, crdErr) + return status, crdErr } if err := waitForRequiredContourCommonCRDs(ctx, snap); err != nil { - return fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) + waitErr := fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, waitErr) + return status, waitErr } var values map[string]any @@ -70,12 +90,16 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ changed, err := m.Apply(ctx, chartContour, helm.StatePresent, values) if err != nil { - return fmt.Errorf("failed to enable ingress: %w", err) + enableErr := fmt.Errorf("failed to enable ingress: %w", err) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, enableErr) + return status, enableErr } if changed { if err := rolloutRestartContour(ctx, snap, 3); err != nil { - return fmt.Errorf("failed to rollout restart contour to apply ingress: %w", err) + resErr := fmt.Errorf("failed to rollout restart contour to apply ingress: %w", err) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, resErr) + return status, resErr } } @@ -87,17 +111,23 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ "defaultTLSSecret": ingress.GetDefaultTLSSecret(), } if _, err := m.Apply(ctx, chartDefaultTLS, helm.StatePresent, values); err != nil { - return fmt.Errorf("failed to install the delegation resource for default TLS secret: %w", err) + tlsErr := fmt.Errorf("failed to install the delegation resource for default TLS secret: %w", err) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, tlsErr) + return status, tlsErr } - return nil + status.Message = enabledMsg + return status, nil } if _, err := m.Apply(ctx, chartDefaultTLS, helm.StateDeleted, nil); err != nil { - return fmt.Errorf("failed to uninstall the delegation resource for default TLS secret: %w", err) + tlsErr := fmt.Errorf("failed to uninstall the delegation resource for default TLS secret: %w", err) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, tlsErr) + return status, tlsErr } - return nil + status.Message = enabledMsg + return status, nil } // applyCommonContourCRDS will install the common contour CRDS when enabled is true. diff --git a/src/k8s/pkg/k8sd/features/coredns/coredns.go b/src/k8s/pkg/k8sd/features/coredns/coredns.go index 08698ccd2..c02985c2c 100644 --- a/src/k8s/pkg/k8sd/features/coredns/coredns.go +++ b/src/k8s/pkg/k8sd/features/coredns/coredns.go @@ -10,19 +10,35 @@ import ( "github.com/canonical/k8s/pkg/snap" ) +const ( + enabledMsg = "enabled" + disabledMsg = "disabled" + deleteFailedMsgTmpl = "Failed to delete DNS, the error was: %v" + deployFailedMsgTmpl = "Failed to deploy DNS, the error was: %v" +) + // ApplyDNS manages the deployment of CoreDNS, with customization options from dns and kubelet, which are retrieved from the cluster configuration. // ApplyDNS will uninstall CoreDNS from the cluster if dns.Enabled is false. // ApplyDNS will install or refresh CoreDNS if dns.Enabled is true. // ApplyDNS will return the ClusterIP address of the coredns service, if successful. -// ApplyDNS returns an error if anything fails. -func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types.Kubelet, _ types.Annotations) (string, error) { +// ApplyDNS will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyDNS returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types.Kubelet, _ types.Annotations) (types.FeatureStatus, string, error) { + status := types.FeatureStatus{Version: imageTag} m := snap.HelmClient() if !dns.GetEnabled() { + status.Enabled = false if _, err := m.Apply(ctx, chart, helm.StateDeleted, nil); err != nil { - return "", fmt.Errorf("failed to uninstall coredns: %w", err) + delErr := fmt.Errorf("failed to uninstall coredns: %w", err) + status.Message = fmt.Sprintf(deleteFailedMsgTmpl, delErr) + return status, "", delErr } - return "", nil + status.Message = disabledMsg + status.Version = "" + return status, "", nil } values := map[string]any{ @@ -64,17 +80,24 @@ func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types. } if _, err := m.Apply(ctx, chart, helm.StatePresent, values); err != nil { - return "", fmt.Errorf("failed to apply coredns: %w", err) + applyErr := fmt.Errorf("failed to apply coredns: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, applyErr) + return status, "", applyErr } client, err := snap.KubernetesClient("") if err != nil { - return "", fmt.Errorf("failed to create kubernetes client: %w", err) + clientErr := fmt.Errorf("failed to create kubernetes client: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, clientErr) + return status, "", clientErr } dnsIP, err := client.GetServiceClusterIP(ctx, "coredns", "kube-system") if err != nil { - return "", fmt.Errorf("failed to retrieve the coredns service: %w", err) + retErr := fmt.Errorf("failed to retrieve the coredns service: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, retErr) + return status, "", retErr } - return dnsIP, nil + status.Message = enabledMsg + return status, dnsIP, nil } diff --git a/src/k8s/pkg/k8sd/features/interface.go b/src/k8s/pkg/k8sd/features/interface.go index 45439f928..18eb4a978 100644 --- a/src/k8s/pkg/k8sd/features/interface.go +++ b/src/k8s/pkg/k8sd/features/interface.go @@ -10,56 +10,56 @@ import ( // Interface abstracts the management of built-in Canonical Kubernetes features. type Interface interface { // ApplyDNS is used to configure the DNS feature on Canonical Kubernetes. - ApplyDNS(context.Context, snap.Snap, types.DNS, types.Kubelet, types.Annotations) (string, error) + ApplyDNS(context.Context, snap.Snap, types.DNS, types.Kubelet, types.Annotations) (types.FeatureStatus, string, error) // ApplyNetwork is used to configure the network feature on Canonical Kubernetes. - ApplyNetwork(context.Context, snap.Snap, types.Network, types.Annotations) error + ApplyNetwork(context.Context, snap.Snap, types.Network, types.Annotations) (types.FeatureStatus, error) // ApplyLoadBalancer is used to configure the load-balancer feature on Canonical Kubernetes. - ApplyLoadBalancer(context.Context, snap.Snap, types.LoadBalancer, types.Network, types.Annotations) error + ApplyLoadBalancer(context.Context, snap.Snap, types.LoadBalancer, types.Network, types.Annotations) (types.FeatureStatus, error) // ApplyIngress is used to configure the ingress controller feature on Canonical Kubernetes. - ApplyIngress(context.Context, snap.Snap, types.Ingress, types.Network, types.Annotations) error + ApplyIngress(context.Context, snap.Snap, types.Ingress, types.Network, types.Annotations) (types.FeatureStatus, error) // ApplyGateway is used to configure the gateway feature on Canonical Kubernetes. - ApplyGateway(context.Context, snap.Snap, types.Gateway, types.Network, types.Annotations) error + ApplyGateway(context.Context, snap.Snap, types.Gateway, types.Network, types.Annotations) (types.FeatureStatus, error) // ApplyMetricsServer is used to configure the metrics-server feature on Canonical Kubernetes. - ApplyMetricsServer(context.Context, snap.Snap, types.MetricsServer, types.Annotations) error + ApplyMetricsServer(context.Context, snap.Snap, types.MetricsServer, types.Annotations) (types.FeatureStatus, error) // ApplyLocalStorage is used to configure the Local Storage feature on Canonical Kubernetes. - ApplyLocalStorage(context.Context, snap.Snap, types.LocalStorage, types.Annotations) error + ApplyLocalStorage(context.Context, snap.Snap, types.LocalStorage, types.Annotations) (types.FeatureStatus, error) } // implementation implements Interface. type implementation struct { - applyDNS func(context.Context, snap.Snap, types.DNS, types.Kubelet, types.Annotations) (string, error) - applyNetwork func(context.Context, snap.Snap, types.Network, types.Annotations) error - applyLoadBalancer func(context.Context, snap.Snap, types.LoadBalancer, types.Network, types.Annotations) error - applyIngress func(context.Context, snap.Snap, types.Ingress, types.Network, types.Annotations) error - applyGateway func(context.Context, snap.Snap, types.Gateway, types.Network, types.Annotations) error - applyMetricsServer func(context.Context, snap.Snap, types.MetricsServer, types.Annotations) error - applyLocalStorage func(context.Context, snap.Snap, types.LocalStorage, types.Annotations) error + applyDNS func(context.Context, snap.Snap, types.DNS, types.Kubelet, types.Annotations) (types.FeatureStatus, string, error) + applyNetwork func(context.Context, snap.Snap, types.Network, types.Annotations) (types.FeatureStatus, error) + applyLoadBalancer func(context.Context, snap.Snap, types.LoadBalancer, types.Network, types.Annotations) (types.FeatureStatus, error) + applyIngress func(context.Context, snap.Snap, types.Ingress, types.Network, types.Annotations) (types.FeatureStatus, error) + applyGateway func(context.Context, snap.Snap, types.Gateway, types.Network, types.Annotations) (types.FeatureStatus, error) + applyMetricsServer func(context.Context, snap.Snap, types.MetricsServer, types.Annotations) (types.FeatureStatus, error) + applyLocalStorage func(context.Context, snap.Snap, types.LocalStorage, types.Annotations) (types.FeatureStatus, error) } -func (i *implementation) ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types.Kubelet, annotations types.Annotations) (string, error) { +func (i *implementation) ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types.Kubelet, annotations types.Annotations) (types.FeatureStatus, string, error) { return i.applyDNS(ctx, snap, dns, kubelet, annotations) } -func (i *implementation) ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annotations types.Annotations) error { +func (i *implementation) ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annotations types.Annotations) (types.FeatureStatus, error) { return i.applyNetwork(ctx, snap, cfg, annotations) } -func (i *implementation) ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, annotations types.Annotations) error { +func (i *implementation) ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, annotations types.Annotations) (types.FeatureStatus, error) { return i.applyLoadBalancer(ctx, snap, loadbalancer, network, annotations) } -func (i *implementation) ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, network types.Network, annotations types.Annotations) error { +func (i *implementation) ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, network types.Network, annotations types.Annotations) (types.FeatureStatus, error) { return i.applyIngress(ctx, snap, ingress, network, annotations) } -func (i *implementation) ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, annotations types.Annotations) error { +func (i *implementation) ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, annotations types.Annotations) (types.FeatureStatus, error) { return i.applyGateway(ctx, snap, gateway, network, annotations) } -func (i *implementation) ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsServer, annotations types.Annotations) error { +func (i *implementation) ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsServer, annotations types.Annotations) (types.FeatureStatus, error) { return i.applyMetricsServer(ctx, snap, cfg, annotations) } -func (i *implementation) ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStorage, annotations types.Annotations) error { +func (i *implementation) ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStorage, annotations types.Annotations) (types.FeatureStatus, error) { return i.applyLocalStorage(ctx, snap, cfg, annotations) } diff --git a/src/k8s/pkg/k8sd/features/localpv/localpv.go b/src/k8s/pkg/k8sd/features/localpv/localpv.go index eb5733056..b20c96395 100644 --- a/src/k8s/pkg/k8sd/features/localpv/localpv.go +++ b/src/k8s/pkg/k8sd/features/localpv/localpv.go @@ -2,16 +2,31 @@ package localpv import ( "context" + "fmt" "github.com/canonical/k8s/pkg/client/helm" "github.com/canonical/k8s/pkg/k8sd/types" "github.com/canonical/k8s/pkg/snap" ) +const ( + enabledMsg = "enabled" + disabledMsg = "disabled" + deployFailedMsgTmpl = "Failed to deploy Local Storage, the error was: %v" + deleteFailedMsgTmpl = "Failed to delete Local Storage, the error was: %v" +) + // ApplyLocalStorage deploys the rawfile-localpv CSI driver on the cluster based on the given configuration, when cfg.Enabled is true. // ApplyLocalStorage removes the rawfile-localpv when cfg.Enabled is false. -// ApplyLocalStorage returns an error if anything fails. -func ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStorage, _ types.Annotations) error { +// ApplyLocalStorage will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyLocalStorage returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStorage, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: imageTag, + Enabled: cfg.GetEnabled(), + } m := snap.HelmClient() values := map[string]any{ @@ -48,5 +63,24 @@ func ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStora } _, err := m.Apply(ctx, chart, helm.StatePresentOrDeleted(cfg.GetEnabled()), values) - return err + if err != nil { + if cfg.GetEnabled() { + enableErr := fmt.Errorf("failed to install rawfile-csi helm package: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + return status, enableErr + } else { + disableErr := fmt.Errorf("failed to delete rawfile-csi helm package: %w", err) + status.Message = fmt.Sprintf(deleteFailedMsgTmpl, disableErr) + return status, disableErr + } + } else { + if cfg.GetEnabled() { + status.Message = enabledMsg + return status, nil + } else { + status.Version = "" + status.Message = disabledMsg + return status, nil + } + } } diff --git a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go index e556a8984..6ffd8846b 100644 --- a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go @@ -10,18 +10,42 @@ import ( "github.com/canonical/k8s/pkg/utils/control" ) -func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, _ types.Annotations) error { +const ( + enabledMsg = "enabled" + disabledMsg = "disabled" + deleteFailedMsgTmpl = "Failed to delete MetalLB, the error was: %v" + deployFailedMsgTmpl = "Failed to deploy MetalLB, the error was: %v" +) + +// ApplyLoadBalancer will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyLoadBalancer returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: controllerImageTag, + Enabled: loadbalancer.GetEnabled(), + } + if !loadbalancer.GetEnabled() { if err := disableLoadBalancer(ctx, snap, network); err != nil { - return fmt.Errorf("failed to disable LoadBalancer: %w", err) + disableErr := fmt.Errorf("failed to disable LoadBalancer: %w", err) + status.Message = fmt.Sprintf(deleteFailedMsgTmpl, disableErr) + return status, disableErr } - return nil + status.Message = disabledMsg + status.Version = "" + return status, nil } if err := enableLoadBalancer(ctx, snap, loadbalancer, network); err != nil { - return fmt.Errorf("failed to enable LoadBalancer: %w", err) + enableErr := fmt.Errorf("failed to enable LoadBalancer: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + return status, enableErr } - return nil + + status.Message = enabledMsg + return status, nil } func disableLoadBalancer(ctx context.Context, snap snap.Snap, network types.Network) error { diff --git a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go index 224db024e..63040726f 100644 --- a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go +++ b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go @@ -2,16 +2,31 @@ package metrics_server import ( "context" + "fmt" "github.com/canonical/k8s/pkg/client/helm" "github.com/canonical/k8s/pkg/k8sd/types" "github.com/canonical/k8s/pkg/snap" ) +const ( + enabledMsg = "enabled" + disabledMsg = "disabled" + deleteFailedMsgTmpl = "Failed to delete Metrics Server, the error was: %v" + deployFailedMsgTmpl = "Failed to deploy Metrics Server, the error was: %v" +) + // ApplyMetricsServer deploys metrics-server when cfg.Enabled is true. // ApplyMetricsServer removes metrics-server when cfg.Enabled is false. -// ApplyMetricsServer returns an error if anything fails. -func ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsServer, annotations types.Annotations) error { +// ApplyMetricsServer will always return a FeatureStatus indicating the current status of the +// deployment. +// ApplyMetricsServer returns an error if anything fails. The error is also wrapped in the .Message field of the +// returned FeatureStatus. +func ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsServer, annotations types.Annotations) (types.FeatureStatus, error) { + status := types.FeatureStatus{ + Version: imageTag, + Enabled: cfg.GetEnabled(), + } m := snap.HelmClient() config := internalConfig(annotations) @@ -28,5 +43,23 @@ func ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsSe } _, err := m.Apply(ctx, chart, helm.StatePresentOrDeleted(cfg.GetEnabled()), values) - return err + if err != nil { + if cfg.GetEnabled() { + enableErr := fmt.Errorf("failed to install metrics server chart: %w", err) + status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + return status, enableErr + } else { + disableErr := fmt.Errorf("failed to delete metrics server chart: %w", err) + status.Message = fmt.Sprintf(deleteFailedMsgTmpl, disableErr) + return status, disableErr + } + } else { + if cfg.GetEnabled() { + status.Message = enabledMsg + return status, nil + } else { + status.Message = disabledMsg + return status, nil + } + } } diff --git a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server_test.go b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server_test.go index c537211c4..ced59104f 100644 --- a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server_test.go +++ b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server_test.go @@ -43,7 +43,7 @@ func TestApplyMetricsServer(t *testing.T) { }, } - err := metrics_server.ApplyMetricsServer(context.Background(), s, tc.config, nil) + status, err := metrics_server.ApplyMetricsServer(context.Background(), s, tc.config, nil) g.Expect(err).ToNot(HaveOccurred()) g.Expect(h.ApplyCalledWith).To(ConsistOf(SatisfyAll( @@ -51,6 +51,11 @@ func TestApplyMetricsServer(t *testing.T) { HaveField("Chart.Namespace", Equal("kube-system")), HaveField("State", Equal(tc.expectState)), ))) + if tc.config.GetEnabled() { + g.Expect(status.Message).To(Equal("enabled")) + } else { + g.Expect(status.Message).To(Equal("disabled")) + } }) } @@ -71,11 +76,12 @@ func TestApplyMetricsServer(t *testing.T) { "k8sd/v1alpha1/metrics-server/image-tag": "custom-tag", } - err := metrics_server.ApplyMetricsServer(context.Background(), s, cfg, annotations) + status, err := metrics_server.ApplyMetricsServer(context.Background(), s, cfg, annotations) g.Expect(err).To(BeNil()) g.Expect(h.ApplyCalledWith).To(ConsistOf(HaveField("Values", HaveKeyWithValue("image", SatisfyAll( HaveKeyWithValue("repository", "custom-image"), HaveKeyWithValue("tag", "custom-tag"), ))))) + g.Expect(status.Message).To(Equal("enabled")) }) } From c5f300188fb455b222ab348328e1e6cf9d5f46b2 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 11:46:13 +0400 Subject: [PATCH 04/19] Update k8s status output --- src/k8s/api/v1/types.go | 118 +++++++----------- src/k8s/pkg/k8sd/app/hooks_start.go | 4 +- src/k8s/pkg/k8sd/database/feature_status.go | 4 +- src/k8s/pkg/k8sd/features/cilium/ingress.go | 4 +- .../pkg/k8sd/features/cilium/loadbalancer.go | 11 +- src/k8s/pkg/k8sd/features/coredns/coredns.go | 4 +- src/k8s/pkg/k8sd/features/localpv/localpv.go | 4 +- .../pkg/k8sd/features/metallb/loadbalancer.go | 11 +- src/k8s/pkg/k8sd/types/feature_status.go | 17 ++- src/k8s/pkg/k8sd/types/feature_status_test.go | 8 +- 10 files changed, 91 insertions(+), 94 deletions(-) diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index 808b6c172..f9c784d0f 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -4,8 +4,6 @@ import ( "fmt" "strings" "time" - - "gopkg.in/yaml.v2" ) type ClusterRole string @@ -51,8 +49,8 @@ type FeatureStatus struct { Message string // Version shows the version of the deployed feature. Version string - // Timestamp shows when the last update was done. - Timestamp time.Time + // UpdatedAt shows when the last update was done. + UpdatedAt time.Time } type Datastore struct { @@ -90,63 +88,6 @@ func (c ClusterStatus) HaClusterFormed() bool { // TICS -COV_GO_SUPPRESSED_ERROR // we are just formatting the output for the k8s status command, it is ok to ignore failures from result.WriteString() -func (c ClusterStatus) datastoreToString() string { - result := strings.Builder{} - - // Datastore - if c.Datastore.Type != "" { - result.WriteString(fmt.Sprintf(" type: %s\n", c.Datastore.Type)) - // Datastore URL for external only - if c.Datastore.Type == "external" { - result.WriteString(fmt.Sprintln(" servers:")) - for _, serverURL := range c.Datastore.Servers { - result.WriteString(fmt.Sprintf(" - %s\n", serverURL)) - } - return result.String() - } - } - - // Datastore roles for dqlite - voters := make([]NodeStatus, 0, len(c.Members)) - standBys := make([]NodeStatus, 0, len(c.Members)) - spares := make([]NodeStatus, 0, len(c.Members)) - for _, node := range c.Members { - switch node.DatastoreRole { - case DatastoreRoleVoter: - voters = append(voters, node) - case DatastoreRoleStandBy: - standBys = append(standBys, node) - case DatastoreRoleSpare: - spares = append(spares, node) - } - } - if len(voters) > 0 { - result.WriteString(" voter-nodes:\n") - for _, voter := range voters { - result.WriteString(fmt.Sprintf(" - %s\n", voter.Address)) - } - } else { - result.WriteString(" voter-nodes: none\n") - } - if len(standBys) > 0 { - result.WriteString(" standby-nodes:\n") - for _, standBy := range standBys { - result.WriteString(fmt.Sprintf(" - %s\n", standBy.Address)) - } - } else { - result.WriteString(" standby-nodes: none\n") - } - if len(spares) > 0 { - result.WriteString(" spare-nodes:\n") - for _, spare := range spares { - result.WriteString(fmt.Sprintf(" - %s\n", spare.Address)) - } - } else { - result.WriteString(" spare-nodes: none\n") - } - - return result.String() -} // TODO: Print k8s version. However, multiple nodes can run different version, so we would need to query all nodes. func (c ClusterStatus) String() string { @@ -154,10 +95,22 @@ func (c ClusterStatus) String() string { // Status if c.Ready { - result.WriteString("status: ready") + result.WriteString("cluster status: ready") } else { - result.WriteString("status: not ready") + result.WriteString("cluster status: not ready") + } + result.WriteString("\n") + + // Control Plane Nodes + result.WriteString("control plane nodes: ") + addrMap := c.getCPNodeAddrToRoleMap() + nodes := make([]string, len(addrMap)) + i := 0 + for addr, role := range addrMap { + nodes[i] = fmt.Sprintf("%s (%s)", addr, role) + i++ } + result.WriteString(strings.Join(nodes, ", ")) result.WriteString("\n") // High availability @@ -167,18 +120,41 @@ func (c ClusterStatus) String() string { } else { result.WriteString("no") } + result.WriteString("\n") // Datastore - result.WriteString("\n") - result.WriteString("datastore:\n") - result.WriteString(c.datastoreToString()) + result.WriteString(fmt.Sprintf("datastore: %s\n", c.Datastore.Type)) + + // Network + result.WriteString(fmt.Sprintf("network: %s\n", c.Network.Message)) + + // DNS + result.WriteString(fmt.Sprintf("dns: %s\n", c.DNS.Message)) + + // Ingress + result.WriteString(fmt.Sprintf("ingress: %s\n", c.Ingress.Message)) + + // Load Balancer + result.WriteString(fmt.Sprintf("load-balancer: %s\n", c.LoadBalancer.Message)) + + // Local Storage + result.WriteString(fmt.Sprintf("local-storage: %s\n", c.LocalStorage.Message)) + + // Gateway + result.WriteString(fmt.Sprintf("gateway: %s\n", c.Gateway.Message)) - // Config - if !c.Config.Empty() { - b, _ := yaml.Marshal(c.Config) - result.WriteString(string(b)) - } return result.String() } // TICS +COV_GO_SUPPRESSED_ERROR + +func (c ClusterStatus) getCPNodeAddrToRoleMap() map[string]string { + m := make(map[string]string) + for _, n := range c.Members { + if n.ClusterRole == ClusterRoleControlPlane { + m[n.Address] = string(n.DatastoreRole) + } + } + + return m +} diff --git a/src/k8s/pkg/k8sd/app/hooks_start.go b/src/k8s/pkg/k8sd/app/hooks_start.go index c4416a207..25efc3508 100644 --- a/src/k8s/pkg/k8sd/app/hooks_start.go +++ b/src/k8s/pkg/k8sd/app/hooks_start.go @@ -76,9 +76,9 @@ func (a *App) onStart(s *state.State) error { func(ctx context.Context, name string, featureStatus types.FeatureStatus) error { if err := s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { // we set timestamp here in order to reduce the clutter. otherwise we will need to - // set .Timestamp field in a lot of places for every event/error. + // set .UpdatedAt field in a lot of places for every event/error. // this is not 100% accurate but should be good enough - featureStatus.Timestamp = time.Now() + featureStatus.UpdatedAt = time.Now() if err := database.SetFeatureStatus(ctx, tx, name, featureStatus); err != nil { return fmt.Errorf("failed to set feature status in db for '%s': %w", name, err) } diff --git a/src/k8s/pkg/k8sd/database/feature_status.go b/src/k8s/pkg/k8sd/database/feature_status.go index 90fb6218b..216cc09cf 100644 --- a/src/k8s/pkg/k8sd/database/feature_status.go +++ b/src/k8s/pkg/k8sd/database/feature_status.go @@ -29,7 +29,7 @@ func SetFeatureStatus(ctx context.Context, tx *sql.Tx, name string, status types name, status.Message, status.Version, - status.Timestamp.Format(time.RFC3339), + status.UpdatedAt.Format(time.RFC3339), status.Enabled, ); err != nil { return fmt.Errorf("failed to execute upsert statement: %w", err) @@ -63,7 +63,7 @@ func GetFeatureStatuses(ctx context.Context, tx *sql.Tx) (map[string]types.Featu return nil, fmt.Errorf("failed to scan row: %w", err) } - typ.Timestamp, err = time.Parse(time.RFC3339, ts) + typ.UpdatedAt, err = time.Parse(time.RFC3339, ts) if err != nil { return nil, fmt.Errorf("failed to parse timestamp: %w", err) } diff --git a/src/k8s/pkg/k8sd/features/cilium/ingress.go b/src/k8s/pkg/k8sd/features/cilium/ingress.go index 49b1d1aab..50106a1c2 100644 --- a/src/k8s/pkg/k8sd/features/cilium/ingress.go +++ b/src/k8s/pkg/k8sd/features/cilium/ingress.go @@ -55,11 +55,11 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, ne if err != nil { if network.GetEnabled() { enableErr := fmt.Errorf("failed to enable ingress: %w", err) - status.Message = fmt.Sprint(ingressDeployFailedMsgTmpl, enableErr) + status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, enableErr) return status, enableErr } else { disableErr := fmt.Errorf("failed to disable ingress: %w", err) - status.Message = fmt.Sprint(ingressDeleteFailedMsgTmpl, disableErr) + status.Message = fmt.Sprintf(ingressDeleteFailedMsgTmpl, disableErr) return status, disableErr } } diff --git a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go index 7ce5b4a3e..fb0e36c40 100644 --- a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go @@ -11,6 +11,7 @@ import ( ) const ( + lbEnabledMsgTmpl = "enabled, %s mode" lbDeleteFailedMsgTmpl = "Failed to delete Cilium Load Balancer, the error was: %v" lbDeployFailedMsgTmpl = "Failed to deploy Cilium Load Balancer, the error was: %v" ) @@ -45,7 +46,15 @@ func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.L status.Message = fmt.Sprintf(lbDeployFailedMsgTmpl, enableErr) return status, enableErr } - status.Message = enabledMsg + + if loadbalancer.GetBGPMode() { + status.Message = fmt.Sprintf(lbEnabledMsgTmpl, "BGP") + } else if loadbalancer.GetL2Mode() { + status.Message = fmt.Sprintf(lbEnabledMsgTmpl, "L2") + } else { + status.Message = fmt.Sprintf(lbEnabledMsgTmpl, "Unknown") + } + return status, nil } diff --git a/src/k8s/pkg/k8sd/features/coredns/coredns.go b/src/k8s/pkg/k8sd/features/coredns/coredns.go index c02985c2c..2bff221d0 100644 --- a/src/k8s/pkg/k8sd/features/coredns/coredns.go +++ b/src/k8s/pkg/k8sd/features/coredns/coredns.go @@ -11,7 +11,7 @@ import ( ) const ( - enabledMsg = "enabled" + enabledMsgTmpl = "enabled at %s" disabledMsg = "disabled" deleteFailedMsgTmpl = "Failed to delete DNS, the error was: %v" deployFailedMsgTmpl = "Failed to deploy DNS, the error was: %v" @@ -98,6 +98,6 @@ func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types. return status, "", retErr } - status.Message = enabledMsg + status.Message = fmt.Sprintf(enabledMsgTmpl, dnsIP) return status, dnsIP, nil } diff --git a/src/k8s/pkg/k8sd/features/localpv/localpv.go b/src/k8s/pkg/k8sd/features/localpv/localpv.go index b20c96395..fda7eb5f1 100644 --- a/src/k8s/pkg/k8sd/features/localpv/localpv.go +++ b/src/k8s/pkg/k8sd/features/localpv/localpv.go @@ -10,7 +10,7 @@ import ( ) const ( - enabledMsg = "enabled" + enabledMsg = "enabled at %s" disabledMsg = "disabled" deployFailedMsgTmpl = "Failed to deploy Local Storage, the error was: %v" deleteFailedMsgTmpl = "Failed to delete Local Storage, the error was: %v" @@ -75,7 +75,7 @@ func ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStora } } else { if cfg.GetEnabled() { - status.Message = enabledMsg + status.Message = fmt.Sprintf(enabledMsg, cfg.GetLocalPath()) return status, nil } else { status.Version = "" diff --git a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go index 6ffd8846b..5c37feaf0 100644 --- a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go @@ -11,7 +11,7 @@ import ( ) const ( - enabledMsg = "enabled" + enabledMsgTmpl = "enabled, %s mode" disabledMsg = "disabled" deleteFailedMsgTmpl = "Failed to delete MetalLB, the error was: %v" deployFailedMsgTmpl = "Failed to deploy MetalLB, the error was: %v" @@ -44,7 +44,14 @@ func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.L return status, enableErr } - status.Message = enabledMsg + if loadbalancer.GetBGPMode() { + status.Message = fmt.Sprintf(enabledMsgTmpl, "BGP") + } else if loadbalancer.GetL2Mode() { + status.Message = fmt.Sprintf(enabledMsgTmpl, "L2") + } else { + status.Message = fmt.Sprintf(enabledMsgTmpl, "Unknown") + } + return status, nil } diff --git a/src/k8s/pkg/k8sd/types/feature_status.go b/src/k8s/pkg/k8sd/types/feature_status.go index 7d886d39b..6ee7e14a6 100644 --- a/src/k8s/pkg/k8sd/types/feature_status.go +++ b/src/k8s/pkg/k8sd/types/feature_status.go @@ -6,11 +6,16 @@ import ( apiv1 "github.com/canonical/k8s/api/v1" ) +// FeatureStatus encapsulates the deployment status of a feature. type FeatureStatus struct { - Enabled bool - Message string - Version string - Timestamp time.Time + // Enabled shows whether or not the deployment of manifests for a status was successful. + Enabled bool + // Message contains information about the status of a feature. It is only supposed to be human readable and informative and should not be programmatically parsed. + Message string + // Version shows the version of the deployed feature. + Version string + // UpdatedAt shows when the last update was done. + UpdatedAt time.Time } func (f FeatureStatus) ToAPI() (apiv1.FeatureStatus, error) { @@ -18,7 +23,7 @@ func (f FeatureStatus) ToAPI() (apiv1.FeatureStatus, error) { Enabled: f.Enabled, Message: f.Message, Version: f.Version, - Timestamp: f.Timestamp, + UpdatedAt: f.UpdatedAt, }, nil } @@ -27,6 +32,6 @@ func FeatureStatusFromAPI(apiFS apiv1.FeatureStatus) (FeatureStatus, error) { Enabled: apiFS.Enabled, Message: apiFS.Message, Version: apiFS.Version, - Timestamp: apiFS.Timestamp, + UpdatedAt: apiFS.UpdatedAt, }, nil } diff --git a/src/k8s/pkg/k8sd/types/feature_status_test.go b/src/k8s/pkg/k8sd/types/feature_status_test.go index 623062e06..17449491e 100644 --- a/src/k8s/pkg/k8sd/types/feature_status_test.go +++ b/src/k8s/pkg/k8sd/types/feature_status_test.go @@ -16,7 +16,7 @@ func TestK8sdFeatureStatusToAPI(t *testing.T) { Enabled: true, Message: "message", Version: "version", - Timestamp: time.Now(), + UpdatedAt: time.Now(), } apiFS, err := k8sdFS.ToAPI() @@ -24,7 +24,7 @@ func TestK8sdFeatureStatusToAPI(t *testing.T) { assert.Equal(t, k8sdFS.Enabled, apiFS.Enabled) assert.Equal(t, k8sdFS.Message, apiFS.Message) assert.Equal(t, k8sdFS.Version, apiFS.Version) - assert.Equal(t, k8sdFS.Timestamp, apiFS.Timestamp) + assert.Equal(t, k8sdFS.UpdatedAt, apiFS.UpdatedAt) } func TestAPIFeatureStatusToK8sd(t *testing.T) { @@ -32,7 +32,7 @@ func TestAPIFeatureStatusToK8sd(t *testing.T) { Enabled: true, Message: "message", Version: "version", - Timestamp: time.Now(), + UpdatedAt: time.Now(), } k8sdFS, err := types.FeatureStatusFromAPI(apiFS) @@ -40,5 +40,5 @@ func TestAPIFeatureStatusToK8sd(t *testing.T) { assert.Equal(t, apiFS.Enabled, k8sdFS.Enabled) assert.Equal(t, apiFS.Message, k8sdFS.Message) assert.Equal(t, apiFS.Version, k8sdFS.Version) - assert.Equal(t, apiFS.Timestamp, k8sdFS.Timestamp) + assert.Equal(t, apiFS.UpdatedAt, k8sdFS.UpdatedAt) } From cbfa67762b8ba3a7ecf0a8cb56df2b73530d47c7 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 12:55:05 +0400 Subject: [PATCH 05/19] Update k8s status output, Fix types test --- src/k8s/api/v1/types.go | 46 +++++++++---- src/k8s/api/v1/types_test.go | 91 +++++++++++++------------ src/k8s/pkg/k8sd/controllers/feature.go | 2 +- 3 files changed, 81 insertions(+), 58 deletions(-) diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index f9c784d0f..a07861f88 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -53,6 +53,16 @@ type FeatureStatus struct { UpdatedAt time.Time } +func (f FeatureStatus) GetMessage() string { + if f.Message != "" { + return f.Message + } + if f.Enabled { + return "enabled" + } + return "disabled" +} + type Datastore struct { Type string `json:"type,omitempty"` Servers []string `json:"servers,omitempty" yaml:"servers,omitempty"` @@ -93,16 +103,19 @@ func (c ClusterStatus) HaClusterFormed() bool { func (c ClusterStatus) String() string { result := strings.Builder{} + // longer than the longest key (eye-balled), make the output left aligned + maxLen := 25 + // Status if c.Ready { - result.WriteString("cluster status: ready") + result.WriteString(fmt.Sprintf("%-*s %s", maxLen, "cluster status:", "ready")) } else { - result.WriteString("cluster status: not ready") + result.WriteString(fmt.Sprintf("%-*s %s", maxLen, "cluster status:", "not ready")) } result.WriteString("\n") // Control Plane Nodes - result.WriteString("control plane nodes: ") + result.WriteString(fmt.Sprintf("%-*s ", maxLen, "control plane nodes:")) addrMap := c.getCPNodeAddrToRoleMap() nodes := make([]string, len(addrMap)) i := 0 @@ -110,11 +123,15 @@ func (c ClusterStatus) String() string { nodes[i] = fmt.Sprintf("%s (%s)", addr, role) i++ } - result.WriteString(strings.Join(nodes, ", ")) + if len(nodes) > 0 { + result.WriteString(strings.Join(nodes, ", ")) + } else { + result.WriteString("none") + } result.WriteString("\n") // High availability - result.WriteString("high-availability: ") + result.WriteString(fmt.Sprintf("%-*s ", maxLen, "high availability:")) if c.HaClusterFormed() { result.WriteString("yes") } else { @@ -123,25 +140,30 @@ func (c ClusterStatus) String() string { result.WriteString("\n") // Datastore - result.WriteString(fmt.Sprintf("datastore: %s\n", c.Datastore.Type)) + // TODO: how to understand if the ds is running or not? + if c.Datastore.Type != "" { + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "datastore:", c.Datastore.Type)) + } else { + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "datastore:", "disabled")) + } // Network - result.WriteString(fmt.Sprintf("network: %s\n", c.Network.Message)) + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "network:", c.Network.GetMessage())) // DNS - result.WriteString(fmt.Sprintf("dns: %s\n", c.DNS.Message)) + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "dns:", c.DNS.GetMessage())) // Ingress - result.WriteString(fmt.Sprintf("ingress: %s\n", c.Ingress.Message)) + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "ingress:", c.Ingress.GetMessage())) // Load Balancer - result.WriteString(fmt.Sprintf("load-balancer: %s\n", c.LoadBalancer.Message)) + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "load-balancer:", c.LoadBalancer.GetMessage())) // Local Storage - result.WriteString(fmt.Sprintf("local-storage: %s\n", c.LocalStorage.Message)) + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "local-storage:", c.LocalStorage.GetMessage())) // Gateway - result.WriteString(fmt.Sprintf("gateway: %s\n", c.Gateway.Message)) + result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "gateway", c.Gateway.GetMessage())) return result.String() } diff --git a/src/k8s/api/v1/types_test.go b/src/k8s/api/v1/types_test.go index b220e3020..0b91e76f8 100644 --- a/src/k8s/api/v1/types_test.go +++ b/src/k8s/api/v1/types_test.go @@ -1,10 +1,10 @@ package apiv1_test import ( + "fmt" "testing" apiv1 "github.com/canonical/k8s/api/v1" - "github.com/canonical/k8s/pkg/utils" . "github.com/onsi/gomega" ) @@ -65,30 +65,28 @@ func TestString(t *testing.T) { clusterStatus: apiv1.ClusterStatus{ Ready: true, Members: []apiv1.NodeStatus{ - {Name: "node1", DatastoreRole: apiv1.DatastoreRoleVoter, Address: "192.168.0.1"}, - {Name: "node2", DatastoreRole: apiv1.DatastoreRoleVoter, Address: "192.168.0.2"}, - {Name: "node3", DatastoreRole: apiv1.DatastoreRoleVoter, Address: "192.168.0.3"}, + {Name: "node1", DatastoreRole: apiv1.DatastoreRoleVoter, Address: "192.168.0.1", ClusterRole: apiv1.ClusterRoleControlPlane}, + {Name: "node2", DatastoreRole: apiv1.DatastoreRoleVoter, Address: "192.168.0.2", ClusterRole: apiv1.ClusterRoleControlPlane}, + {Name: "node3", DatastoreRole: apiv1.DatastoreRoleStandBy, Address: "192.168.0.3", ClusterRole: apiv1.ClusterRoleControlPlane}, }, - Config: apiv1.UserFacingClusterConfig{ - Network: apiv1.NetworkConfig{Enabled: utils.Pointer(true)}, - DNS: apiv1.DNSConfig{Enabled: utils.Pointer(true)}, - }, - Datastore: apiv1.Datastore{Type: "k8s-dqlite"}, + Datastore: apiv1.Datastore{Type: "k8s-dqlite"}, + Network: apiv1.FeatureStatus{Message: "enabled"}, + DNS: apiv1.FeatureStatus{Message: "enabled at 192.168.0.10"}, + Ingress: apiv1.FeatureStatus{Message: "enabled"}, + LoadBalancer: apiv1.FeatureStatus{Message: "enabled, L2 mode"}, + LocalStorage: apiv1.FeatureStatus{Message: "enabled at /var/snap/k8s/common/rawfile-storage"}, + Gateway: apiv1.FeatureStatus{Message: "enabled"}, }, - expectedOutput: `status: ready -high-availability: yes -datastore: - type: k8s-dqlite - voter-nodes: - - 192.168.0.1 - - 192.168.0.2 - - 192.168.0.3 - standby-nodes: none - spare-nodes: none -network: - enabled: true -dns: - enabled: true + expectedOutput: `cluster status: ready +control plane nodes: 192.168.0.1 (voter), 192.168.0.2 (voter), 192.168.0.3 (stand-by) +high availability: no +datastore: k8s-dqlite +network: enabled +dns: enabled at 192.168.0.10 +ingress: enabled +load-balancer: enabled, L2 mode +local-storage: enabled at /var/snap/k8s/common/rawfile-storage +gateway enabled `, }, { @@ -96,25 +94,22 @@ dns: clusterStatus: apiv1.ClusterStatus{ Ready: true, Members: []apiv1.NodeStatus{ - {Name: "node1", DatastoreRole: apiv1.DatastoreRoleVoter, Address: "192.168.0.1"}, - }, - Config: apiv1.UserFacingClusterConfig{ - Network: apiv1.NetworkConfig{Enabled: utils.Pointer(true)}, - DNS: apiv1.DNSConfig{Enabled: utils.Pointer(true)}, + {Name: "node1", DatastoreRole: apiv1.DatastoreRoleVoter, Address: "192.168.0.1", ClusterRole: apiv1.ClusterRoleControlPlane}, }, Datastore: apiv1.Datastore{Type: "external", Servers: []string{"etcd-url1", "etcd-url2"}}, + Network: apiv1.FeatureStatus{Message: "enabled"}, + DNS: apiv1.FeatureStatus{Message: "enabled at 192.168.0.10"}, }, - expectedOutput: `status: ready -high-availability: no -datastore: - type: external - servers: - - etcd-url1 - - etcd-url2 -network: - enabled: true -dns: - enabled: true + expectedOutput: `cluster status: ready +control plane nodes: 192.168.0.1 (voter) +high availability: no +datastore: external +network: enabled +dns: enabled at 192.168.0.10 +ingress: disabled +load-balancer: disabled +local-storage: disabled +gateway disabled `, }, { @@ -125,12 +120,16 @@ dns: Config: apiv1.UserFacingClusterConfig{}, Datastore: apiv1.Datastore{}, }, - expectedOutput: `status: not ready -high-availability: no -datastore: - voter-nodes: none - standby-nodes: none - spare-nodes: none + expectedOutput: `cluster status: not ready +control plane nodes: none +high availability: no +datastore: disabled +network: disabled +dns: disabled +ingress: disabled +load-balancer: disabled +local-storage: disabled +gateway disabled `, }, } @@ -138,6 +137,8 @@ datastore: for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) + fmt.Println("######################## cluster status:") + fmt.Println(tc.clusterStatus.String()) g.Expect(tc.clusterStatus.String()).To(Equal(tc.expectedOutput)) }) } diff --git a/src/k8s/pkg/k8sd/controllers/feature.go b/src/k8s/pkg/k8sd/controllers/feature.go index 86397994d..759ea3f0f 100644 --- a/src/k8s/pkg/k8sd/controllers/feature.go +++ b/src/k8s/pkg/k8sd/controllers/feature.go @@ -138,7 +138,7 @@ func (c *FeatureController) reconcile( } if err := setFeatureStatus(ctx, componentName, featureStatus); err != nil { - return fmt.Errorf("failed to set feature status for '%s': %w", err) + return fmt.Errorf("failed to set feature status for '%s': %w", componentName, err) } return nil From b9ba09c68a4367ef898c5b69a3485ca02c3b28fc Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 13:17:25 +0400 Subject: [PATCH 06/19] Fix feature status tests --- .../pkg/k8sd/database/feature_status_test.go | 107 +++++++++++++++++- 1 file changed, 101 insertions(+), 6 deletions(-) diff --git a/src/k8s/pkg/k8sd/database/feature_status_test.go b/src/k8s/pkg/k8sd/database/feature_status_test.go index 5303536a1..bea531c37 100644 --- a/src/k8s/pkg/k8sd/database/feature_status_test.go +++ b/src/k8s/pkg/k8sd/database/feature_status_test.go @@ -1,11 +1,106 @@ package database_test -import "testing" +import ( + "context" + "database/sql" + "testing" + "time" -func TestSetFeatureStatus(t *testing.T) { - t.FailNow() -} + "github.com/canonical/k8s/pkg/k8sd/database" + "github.com/canonical/k8s/pkg/k8sd/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFeatureStatus(t *testing.T) { + WithDB(t, func(ctx context.Context, db DB) { + _ = db.Transaction(ctx, func(ctx context.Context, tx *sql.Tx) error { + t0, _ := time.Parse(time.RFC3339, time.Now().Format(time.RFC3339)) + // initial get should return nothing + ss, err := database.GetFeatureStatuses(ctx, tx) + require.NoError(t, err) + assert.Len(t, ss, 0) + + networkS := types.FeatureStatus{ + Enabled: true, + Message: "enabled", + Version: "1.2.3", + UpdatedAt: t0, + } + dnsS := types.FeatureStatus{ + Enabled: true, + Message: "enabled at 10.0.0.1", + Version: "4.5.6", + UpdatedAt: t0, + } + // setting new values + err = database.SetFeatureStatus(ctx, tx, "network", networkS) + require.NoError(t, err) + err = database.SetFeatureStatus(ctx, tx, "dns", dnsS) + require.NoError(t, err) + + // getting new values + ss, err = database.GetFeatureStatuses(ctx, tx) + require.NoError(t, err) + assert.Len(t, ss, 2) + + assert.Equal(t, networkS.Enabled, ss["network"].Enabled) + assert.Equal(t, networkS.Message, ss["network"].Message) + assert.Equal(t, networkS.Version, ss["network"].Version) + assert.Equal(t, networkS.UpdatedAt, ss["network"].UpdatedAt) + + assert.Equal(t, dnsS.Enabled, ss["dns"].Enabled) + assert.Equal(t, dnsS.Message, ss["dns"].Message) + assert.Equal(t, dnsS.Version, ss["dns"].Version) + assert.Equal(t, dnsS.UpdatedAt, ss["dns"].UpdatedAt) + + // updating old values and adding new ones + dnsS2 := types.FeatureStatus{ + Enabled: true, + Message: "enabled at 10.0.0.2", + Version: "4.5.7", + UpdatedAt: t0, + } + gatewayS := types.FeatureStatus{ + Enabled: true, + Message: "disabled", + Version: "10.20.30", + UpdatedAt: t0, + } + // setting the old value for network again + err = database.SetFeatureStatus(ctx, tx, "network", networkS) + require.NoError(t, err) + // updating dns with new value + err = database.SetFeatureStatus(ctx, tx, "dns", dnsS2) + require.NoError(t, err) + // adding new status + err = database.SetFeatureStatus(ctx, tx, "gateway", gatewayS) + require.NoError(t, err) + + // checking the new values + ss, err = database.GetFeatureStatuses(ctx, tx) + require.NoError(t, err) + assert.Len(t, ss, 3) + + // network stayed the same + assert.Equal(t, networkS.Enabled, ss["network"].Enabled) + assert.Equal(t, networkS.Message, ss["network"].Message) + assert.Equal(t, networkS.Version, ss["network"].Version) + assert.Equal(t, networkS.UpdatedAt, ss["network"].UpdatedAt) + + // dns is updated + assert.Equal(t, dnsS2.Enabled, ss["dns"].Enabled) + assert.Equal(t, dnsS2.Message, ss["dns"].Message) + assert.Equal(t, dnsS2.Version, ss["dns"].Version) + assert.Equal(t, dnsS2.UpdatedAt, ss["dns"].UpdatedAt) + + // gateway is added + assert.Equal(t, gatewayS.Enabled, ss["gateway"].Enabled) + assert.Equal(t, gatewayS.Message, ss["gateway"].Message) + assert.Equal(t, gatewayS.Version, ss["gateway"].Version) + assert.Equal(t, gatewayS.UpdatedAt, ss["gateway"].UpdatedAt) -func TestGetFeatureStatuses(t *testing.T) { - t.FailNow() + return nil + }) + }) } From 975b33b864ea1fa3f19c678768e88ab0dc72757c Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 13:20:47 +0400 Subject: [PATCH 07/19] Reorder setFeatureStatus param --- src/k8s/pkg/k8sd/controllers/feature.go | 34 ++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/k8s/pkg/k8sd/controllers/feature.go b/src/k8s/pkg/k8sd/controllers/feature.go index 759ea3f0f..aae1d6315 100644 --- a/src/k8s/pkg/k8sd/controllers/feature.go +++ b/src/k8s/pkg/k8sd/controllers/feature.go @@ -78,31 +78,31 @@ func (c *FeatureController) Run( c.waitReady() ctx = log.NewContext(ctx, log.FromContext(ctx).WithValues("controller", "feature")) - go c.reconcileLoop(ctx, getClusterConfig, "network", c.triggerNetworkCh, c.reconciledNetworkCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + go c.reconcileLoop(ctx, getClusterConfig, setFeatureStatus, "network", c.triggerNetworkCh, c.reconciledNetworkCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyNetwork(ctx, c.snap, cfg.Network, cfg.Annotations) - }, setFeatureStatus) + }) - go c.reconcileLoop(ctx, getClusterConfig, "gateway", c.triggerGatewayCh, c.reconciledGatewayCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + go c.reconcileLoop(ctx, getClusterConfig, setFeatureStatus, "gateway", c.triggerGatewayCh, c.reconciledGatewayCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyGateway(ctx, c.snap, cfg.Gateway, cfg.Network, cfg.Annotations) - }, setFeatureStatus) + }) - go c.reconcileLoop(ctx, getClusterConfig, "ingress", c.triggerIngressCh, c.reconciledIngressCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + go c.reconcileLoop(ctx, getClusterConfig, setFeatureStatus, "ingress", c.triggerIngressCh, c.reconciledIngressCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyIngress(ctx, c.snap, cfg.Ingress, cfg.Network, cfg.Annotations) - }, setFeatureStatus) + }) - go c.reconcileLoop(ctx, getClusterConfig, "load-balancer", c.triggerLoadBalancerCh, c.reconciledLoadBalancerCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + go c.reconcileLoop(ctx, getClusterConfig, setFeatureStatus, "load-balancer", c.triggerLoadBalancerCh, c.reconciledLoadBalancerCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyLoadBalancer(ctx, c.snap, cfg.LoadBalancer, cfg.Network, cfg.Annotations) - }, setFeatureStatus) + }) - go c.reconcileLoop(ctx, getClusterConfig, "local-storage", c.triggerLocalStorageCh, c.reconciledLocalStorageCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + go c.reconcileLoop(ctx, getClusterConfig, setFeatureStatus, "local-storage", c.triggerLocalStorageCh, c.reconciledLocalStorageCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyLocalStorage(ctx, c.snap, cfg.LocalStorage, cfg.Annotations) - }, setFeatureStatus) + }) - go c.reconcileLoop(ctx, getClusterConfig, "metrics-server", c.triggerMetricsServerCh, c.reconciledMetricsServerCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + go c.reconcileLoop(ctx, getClusterConfig, setFeatureStatus, "metrics-server", c.triggerMetricsServerCh, c.reconciledMetricsServerCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { return features.Implementation.ApplyMetricsServer(ctx, c.snap, cfg.MetricsServer, cfg.Annotations) - }, setFeatureStatus) + }) - go c.reconcileLoop(ctx, getClusterConfig, "dns", c.triggerDNSCh, c.reconciledDNSCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { + go c.reconcileLoop(ctx, getClusterConfig, setFeatureStatus, "dns", c.triggerDNSCh, c.reconciledDNSCh, func(cfg types.ClusterConfig) (types.FeatureStatus, error) { featureStatus, dnsIP, err := features.Implementation.ApplyDNS(ctx, c.snap, cfg.DNS, cfg.Kubelet, cfg.Annotations) if err != nil { @@ -117,15 +117,15 @@ func (c *FeatureController) Run( } } return featureStatus, nil - }, setFeatureStatus) + }) } func (c *FeatureController) reconcile( ctx context.Context, getClusterConfig func(context.Context) (types.ClusterConfig, error), + setFeatureStatus func(context.Context, string, types.FeatureStatus) error, apply func(cfg types.ClusterConfig) (types.FeatureStatus, error), componentName string, - setFeatureStatus func(context.Context, string, types.FeatureStatus) error, ) error { cfg, err := getClusterConfig(ctx) if err != nil { @@ -147,18 +147,18 @@ func (c *FeatureController) reconcile( func (c *FeatureController) reconcileLoop( ctx context.Context, getClusterConfig func(context.Context) (types.ClusterConfig, error), + setFeatureStatus func(ctx context.Context, name string, status types.FeatureStatus) error, componentName string, triggerCh chan struct{}, reconciledCh chan<- struct{}, apply func(cfg types.ClusterConfig) (types.FeatureStatus, error), - setFeatureStatus func(ctx context.Context, name string, status types.FeatureStatus) error, ) { for { select { case <-ctx.Done(): return case <-triggerCh: - if err := c.reconcile(ctx, getClusterConfig, apply, componentName, setFeatureStatus); err != nil { + if err := c.reconcile(ctx, getClusterConfig, setFeatureStatus, apply, componentName); err != nil { log.FromContext(ctx).WithValues("feature", componentName).Error(err, "Failed to apply feature configuration") // notify triggerCh after 5 seconds to retry From 4d41768f66d080e5df1f70f6ef250d168e8988e7 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 13:36:09 +0400 Subject: [PATCH 08/19] Update reconcile loop --- src/k8s/pkg/k8sd/app/hooks_start.go | 2 +- src/k8s/pkg/k8sd/controllers/feature.go | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/k8s/pkg/k8sd/app/hooks_start.go b/src/k8s/pkg/k8sd/app/hooks_start.go index 25efc3508..4cad69855 100644 --- a/src/k8s/pkg/k8sd/app/hooks_start.go +++ b/src/k8s/pkg/k8sd/app/hooks_start.go @@ -80,7 +80,7 @@ func (a *App) onStart(s *state.State) error { // this is not 100% accurate but should be good enough featureStatus.UpdatedAt = time.Now() if err := database.SetFeatureStatus(ctx, tx, name, featureStatus); err != nil { - return fmt.Errorf("failed to set feature status in db for '%s': %w", name, err) + return fmt.Errorf("failed to set feature status in db for %q: %w", name, err) } return nil }); err != nil { diff --git a/src/k8s/pkg/k8sd/controllers/feature.go b/src/k8s/pkg/k8sd/controllers/feature.go index aae1d6315..98073b59a 100644 --- a/src/k8s/pkg/k8sd/controllers/feature.go +++ b/src/k8s/pkg/k8sd/controllers/feature.go @@ -123,9 +123,8 @@ func (c *FeatureController) Run( func (c *FeatureController) reconcile( ctx context.Context, getClusterConfig func(context.Context) (types.ClusterConfig, error), - setFeatureStatus func(context.Context, string, types.FeatureStatus) error, apply func(cfg types.ClusterConfig) (types.FeatureStatus, error), - componentName string, + updateFeatureStatus func(context.Context, types.FeatureStatus) error, ) error { cfg, err := getClusterConfig(ctx) if err != nil { @@ -137,8 +136,8 @@ func (c *FeatureController) reconcile( return fmt.Errorf("failed to apply configuration: %w", err) } - if err := setFeatureStatus(ctx, componentName, featureStatus); err != nil { - return fmt.Errorf("failed to set feature status for '%s': %w", componentName, err) + if err := updateFeatureStatus(ctx, featureStatus); err != nil { + return fmt.Errorf("failed to update feature status: %w", err) } return nil @@ -158,7 +157,9 @@ func (c *FeatureController) reconcileLoop( case <-ctx.Done(): return case <-triggerCh: - if err := c.reconcile(ctx, getClusterConfig, setFeatureStatus, apply, componentName); err != nil { + if err := c.reconcile(ctx, getClusterConfig, apply, func(ctx context.Context, status types.FeatureStatus) error { + return setFeatureStatus(ctx, componentName, status) + }); err != nil { log.FromContext(ctx).WithValues("feature", componentName).Error(err, "Failed to apply feature configuration") // notify triggerCh after 5 seconds to retry From ed1cc1cba5ad39bb6ca8418f145f2fae612bce21 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 14:43:07 +0400 Subject: [PATCH 09/19] Replace testify with gomega, Address comments and issues --- src/k8s/api/v1/types.go | 26 ++----- src/k8s/api/v1/types_test.go | 7 +- src/k8s/go.mod | 3 +- src/k8s/pkg/k8sd/api/cluster.go | 30 +++------ src/k8s/pkg/k8sd/database/feature_status.go | 28 ++++---- .../pkg/k8sd/database/feature_status_test.go | 67 ++++++++++--------- .../sql/queries/feature-status/select.sql | 2 +- src/k8s/pkg/k8sd/types/feature_status.go | 8 +-- src/k8s/pkg/k8sd/types/feature_status_test.go | 27 ++++---- 9 files changed, 85 insertions(+), 113 deletions(-) diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index a07861f88..5d2212936 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -116,15 +116,12 @@ func (c ClusterStatus) String() string { // Control Plane Nodes result.WriteString(fmt.Sprintf("%-*s ", maxLen, "control plane nodes:")) - addrMap := c.getCPNodeAddrToRoleMap() - nodes := make([]string, len(addrMap)) - i := 0 - for addr, role := range addrMap { - nodes[i] = fmt.Sprintf("%s (%s)", addr, role) - i++ - } - if len(nodes) > 0 { - result.WriteString(strings.Join(nodes, ", ")) + if len(c.Members) > 0 { + for _, m := range c.Members { + if m.ClusterRole == ClusterRoleControlPlane { + result.WriteString(fmt.Sprintf("%s (%s) ", m.Address, m.DatastoreRole)) + } + } } else { result.WriteString("none") } @@ -169,14 +166,3 @@ func (c ClusterStatus) String() string { } // TICS +COV_GO_SUPPRESSED_ERROR - -func (c ClusterStatus) getCPNodeAddrToRoleMap() map[string]string { - m := make(map[string]string) - for _, n := range c.Members { - if n.ClusterRole == ClusterRoleControlPlane { - m[n.Address] = string(n.DatastoreRole) - } - } - - return m -} diff --git a/src/k8s/api/v1/types_test.go b/src/k8s/api/v1/types_test.go index 0b91e76f8..c7790486e 100644 --- a/src/k8s/api/v1/types_test.go +++ b/src/k8s/api/v1/types_test.go @@ -1,7 +1,6 @@ package apiv1_test import ( - "fmt" "testing" apiv1 "github.com/canonical/k8s/api/v1" @@ -78,7 +77,7 @@ func TestString(t *testing.T) { Gateway: apiv1.FeatureStatus{Message: "enabled"}, }, expectedOutput: `cluster status: ready -control plane nodes: 192.168.0.1 (voter), 192.168.0.2 (voter), 192.168.0.3 (stand-by) +control plane nodes: 192.168.0.1 (voter) 192.168.0.2 (voter) 192.168.0.3 (stand-by) high availability: no datastore: k8s-dqlite network: enabled @@ -101,7 +100,7 @@ gateway enabled DNS: apiv1.FeatureStatus{Message: "enabled at 192.168.0.10"}, }, expectedOutput: `cluster status: ready -control plane nodes: 192.168.0.1 (voter) +control plane nodes: 192.168.0.1 (voter) high availability: no datastore: external network: enabled @@ -137,8 +136,6 @@ gateway disabled for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - fmt.Println("######################## cluster status:") - fmt.Println(tc.clusterStatus.String()) g.Expect(tc.clusterStatus.String()).To(Equal(tc.expectedOutput)) }) } diff --git a/src/k8s/go.mod b/src/k8s/go.mod index efd3bd57b..d6387051a 100644 --- a/src/k8s/go.mod +++ b/src/k8s/go.mod @@ -12,7 +12,6 @@ require ( github.com/onsi/gomega v1.32.0 github.com/pelletier/go-toml v1.9.5 github.com/spf13/cobra v1.8.0 - github.com/stretchr/testify v1.9.0 golang.org/x/net v0.23.0 golang.org/x/sys v0.19.0 gopkg.in/yaml.v2 v2.4.0 @@ -123,7 +122,6 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/pkg/sftp v1.13.6 // indirect github.com/pkg/xattr v0.4.9 // indirect - github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.19.0 // indirect github.com/prometheus/client_model v0.6.0 // indirect github.com/prometheus/common v0.51.1 // indirect @@ -135,6 +133,7 @@ require ( github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/cast v1.6.0 // indirect github.com/spf13/pflag v1.0.5 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xeipuuv/gojsonschema v1.2.0 // indirect diff --git a/src/k8s/pkg/k8sd/api/cluster.go b/src/k8s/pkg/k8sd/api/cluster.go index 9430e6c72..d3837c0e2 100644 --- a/src/k8s/pkg/k8sd/api/cluster.go +++ b/src/k8s/pkg/k8sd/api/cluster.go @@ -10,6 +10,7 @@ import ( "github.com/canonical/k8s/pkg/k8sd/api/impl" "github.com/canonical/k8s/pkg/k8sd/database" databaseutil "github.com/canonical/k8s/pkg/k8sd/database/util" + "github.com/canonical/k8s/pkg/k8sd/types" "github.com/canonical/lxd/lxd/response" "github.com/canonical/microcluster/state" ) @@ -39,22 +40,13 @@ func (e *Endpoints) getClusterStatus(s *state.State, r *http.Request) response.R return response.InternalError(fmt.Errorf("failed to check if cluster has ready nodes: %w", err)) } - featureStatuses := make(map[string]apiv1.FeatureStatus) + var statuses map[string]types.FeatureStatus if err := s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { - statuses, err := database.GetFeatureStatuses(s.Context, tx) + var err error + statuses, err = database.GetFeatureStatuses(s.Context, tx) if err != nil { return fmt.Errorf("failed to get feature statuses: %w", err) } - - for name, st := range statuses { - apiSt, err := st.ToAPI() - if err != nil { - return fmt.Errorf("failed to convert k8sd feature status to api feature status: %w", err) - } - - featureStatuses[name] = apiSt - } - return nil }); err != nil { return response.InternalError(fmt.Errorf("database transaction failed: %w", err)) @@ -69,13 +61,13 @@ func (e *Endpoints) getClusterStatus(s *state.State, r *http.Request) response.R Type: config.Datastore.GetType(), Servers: config.Datastore.GetExternalServers(), }, - DNS: featureStatuses["dns"], - Network: featureStatuses["network"], - LoadBalancer: featureStatuses["load-balancer"], - Ingress: featureStatuses["ingress"], - Gateway: featureStatuses["gateway"], - MetricsServer: featureStatuses["metrics-server"], - LocalStorage: featureStatuses["local-storage"], + DNS: statuses["dns"].ToAPI(), + Network: statuses["network"].ToAPI(), + LoadBalancer: statuses["load-balancer"].ToAPI(), + Ingress: statuses["ingress"].ToAPI(), + Gateway: statuses["gateway"].ToAPI(), + MetricsServer: statuses["metrics-server"].ToAPI(), + LocalStorage: statuses["local-storage"].ToAPI(), }, } diff --git a/src/k8s/pkg/k8sd/database/feature_status.go b/src/k8s/pkg/k8sd/database/feature_status.go index 216cc09cf..b6e7b1f7a 100644 --- a/src/k8s/pkg/k8sd/database/feature_status.go +++ b/src/k8s/pkg/k8sd/database/feature_status.go @@ -7,20 +7,18 @@ import ( "time" "github.com/canonical/k8s/pkg/k8sd/types" + "github.com/canonical/k8s/pkg/log" "github.com/canonical/microcluster/cluster" ) -var featureStatusStmts = struct { - select_ int - upsert_ int -}{ - select_: MustPrepareStatement("feature-status", "select.sql"), - upsert_: MustPrepareStatement("feature-status", "upsert.sql"), +var featureStatusStmts = map[string]int{ + "select": MustPrepareStatement("feature-status", "select.sql"), + "upsert": MustPrepareStatement("feature-status", "upsert.sql"), } // SetFeatureStatus updates the status of the given feature. func SetFeatureStatus(ctx context.Context, tx *sql.Tx, name string, status types.FeatureStatus) error { - upsertTxStmt, err := cluster.Stmt(tx, featureStatusStmts.upsert_) + upsertTxStmt, err := cluster.Stmt(tx, featureStatusStmts["upsert"]) if err != nil { return fmt.Errorf("failed to prepare upsert statement: %w", err) } @@ -40,7 +38,7 @@ func SetFeatureStatus(ctx context.Context, tx *sql.Tx, name string, status types // GetFeatureStatuses returns a map of feature names to their status. func GetFeatureStatuses(ctx context.Context, tx *sql.Tx) (map[string]types.FeatureStatus, error) { - selectTxStmt, err := cluster.Stmt(tx, featureStatusStmts.select_) + selectTxStmt, err := cluster.Stmt(tx, featureStatusStmts["select"]) if err != nil { return nil, fmt.Errorf("failed to prepare select statement: %w", err) } @@ -50,30 +48,30 @@ func GetFeatureStatuses(ctx context.Context, tx *sql.Tx) (map[string]types.Featu return nil, fmt.Errorf("failed to execute select statement: %w", err) } - fsMap := make(map[string]types.FeatureStatus) + result := make(map[string]types.FeatureStatus) for rows.Next() { var ( name string ts string ) - typ := types.FeatureStatus{} + status := types.FeatureStatus{} - if err := rows.Scan(&name, &typ.Message, &typ.Version, &ts, &typ.Enabled); err != nil { + if err := rows.Scan(&name, &status.Message, &status.Version, &ts, &status.Enabled); err != nil { return nil, fmt.Errorf("failed to scan row: %w", err) } - typ.UpdatedAt, err = time.Parse(time.RFC3339, ts) + status.UpdatedAt, err = time.Parse(time.RFC3339, ts) if err != nil { - return nil, fmt.Errorf("failed to parse timestamp: %w", err) + log.L().Error(err, "failed to parse time", "original", ts) } - fsMap[name] = typ + result[name] = status } if rows.Err() != nil { return nil, fmt.Errorf("failed to read rows: %w", err) } - return fsMap, nil + return result, nil } diff --git a/src/k8s/pkg/k8sd/database/feature_status_test.go b/src/k8s/pkg/k8sd/database/feature_status_test.go index bea531c37..cedf1f93c 100644 --- a/src/k8s/pkg/k8sd/database/feature_status_test.go +++ b/src/k8s/pkg/k8sd/database/feature_status_test.go @@ -6,20 +6,21 @@ import ( "testing" "time" + . "github.com/onsi/gomega" + "github.com/canonical/k8s/pkg/k8sd/database" "github.com/canonical/k8s/pkg/k8sd/types" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestFeatureStatus(t *testing.T) { WithDB(t, func(ctx context.Context, db DB) { _ = db.Transaction(ctx, func(ctx context.Context, tx *sql.Tx) error { + g := NewWithT(t) t0, _ := time.Parse(time.RFC3339, time.Now().Format(time.RFC3339)) // initial get should return nothing ss, err := database.GetFeatureStatuses(ctx, tx) - require.NoError(t, err) - assert.Len(t, ss, 0) + g.Expect(err).To(BeNil()) + g.Expect(len(ss)).To(Equal(0)) networkS := types.FeatureStatus{ Enabled: true, @@ -35,24 +36,24 @@ func TestFeatureStatus(t *testing.T) { } // setting new values err = database.SetFeatureStatus(ctx, tx, "network", networkS) - require.NoError(t, err) + g.Expect(err).To(BeNil()) err = database.SetFeatureStatus(ctx, tx, "dns", dnsS) - require.NoError(t, err) + g.Expect(err).To(BeNil()) // getting new values ss, err = database.GetFeatureStatuses(ctx, tx) - require.NoError(t, err) - assert.Len(t, ss, 2) + g.Expect(err).To(BeNil()) + g.Expect(len(ss)).To(Equal(2)) - assert.Equal(t, networkS.Enabled, ss["network"].Enabled) - assert.Equal(t, networkS.Message, ss["network"].Message) - assert.Equal(t, networkS.Version, ss["network"].Version) - assert.Equal(t, networkS.UpdatedAt, ss["network"].UpdatedAt) + g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) + g.Expect(ss["network"].Message).To(Equal(networkS.Message)) + g.Expect(ss["network"].Version).To(Equal(networkS.Version)) + g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) - assert.Equal(t, dnsS.Enabled, ss["dns"].Enabled) - assert.Equal(t, dnsS.Message, ss["dns"].Message) - assert.Equal(t, dnsS.Version, ss["dns"].Version) - assert.Equal(t, dnsS.UpdatedAt, ss["dns"].UpdatedAt) + g.Expect(ss["dns"].Enabled).To(Equal(dnsS.Enabled)) + g.Expect(ss["dns"].Message).To(Equal(dnsS.Message)) + g.Expect(ss["dns"].Version).To(Equal(dnsS.Version)) + g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS.UpdatedAt)) // updating old values and adding new ones dnsS2 := types.FeatureStatus{ @@ -69,36 +70,36 @@ func TestFeatureStatus(t *testing.T) { } // setting the old value for network again err = database.SetFeatureStatus(ctx, tx, "network", networkS) - require.NoError(t, err) + g.Expect(err).To(BeNil()) // updating dns with new value err = database.SetFeatureStatus(ctx, tx, "dns", dnsS2) - require.NoError(t, err) + g.Expect(err).To(BeNil()) // adding new status err = database.SetFeatureStatus(ctx, tx, "gateway", gatewayS) - require.NoError(t, err) + g.Expect(err).To(BeNil()) // checking the new values ss, err = database.GetFeatureStatuses(ctx, tx) - require.NoError(t, err) - assert.Len(t, ss, 3) + g.Expect(err).To(BeNil()) + g.Expect(len(ss)).To(Equal(3)) // network stayed the same - assert.Equal(t, networkS.Enabled, ss["network"].Enabled) - assert.Equal(t, networkS.Message, ss["network"].Message) - assert.Equal(t, networkS.Version, ss["network"].Version) - assert.Equal(t, networkS.UpdatedAt, ss["network"].UpdatedAt) + g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) + g.Expect(ss["network"].Message).To(Equal(networkS.Message)) + g.Expect(ss["network"].Version).To(Equal(networkS.Version)) + g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) // dns is updated - assert.Equal(t, dnsS2.Enabled, ss["dns"].Enabled) - assert.Equal(t, dnsS2.Message, ss["dns"].Message) - assert.Equal(t, dnsS2.Version, ss["dns"].Version) - assert.Equal(t, dnsS2.UpdatedAt, ss["dns"].UpdatedAt) + g.Expect(ss["dns"].Enabled).To(Equal(dnsS2.Enabled)) + g.Expect(ss["dns"].Message).To(Equal(dnsS2.Message)) + g.Expect(ss["dns"].Version).To(Equal(dnsS2.Version)) + g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS2.UpdatedAt)) // gateway is added - assert.Equal(t, gatewayS.Enabled, ss["gateway"].Enabled) - assert.Equal(t, gatewayS.Message, ss["gateway"].Message) - assert.Equal(t, gatewayS.Version, ss["gateway"].Version) - assert.Equal(t, gatewayS.UpdatedAt, ss["gateway"].UpdatedAt) + g.Expect(ss["gateway"].Enabled).To(Equal(gatewayS.Enabled)) + g.Expect(ss["gateway"].Message).To(Equal(gatewayS.Message)) + g.Expect(ss["gateway"].Version).To(Equal(gatewayS.Version)) + g.Expect(ss["gateway"].UpdatedAt).To(Equal(gatewayS.UpdatedAt)) return nil }) diff --git a/src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql b/src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql index d1549ec42..1d7b5e077 100644 --- a/src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql +++ b/src/k8s/pkg/k8sd/database/sql/queries/feature-status/select.sql @@ -1,4 +1,4 @@ SELECT name, message, version, timestamp, enabled FROM - feature_status \ No newline at end of file + feature_status diff --git a/src/k8s/pkg/k8sd/types/feature_status.go b/src/k8s/pkg/k8sd/types/feature_status.go index 6ee7e14a6..162184527 100644 --- a/src/k8s/pkg/k8sd/types/feature_status.go +++ b/src/k8s/pkg/k8sd/types/feature_status.go @@ -18,20 +18,20 @@ type FeatureStatus struct { UpdatedAt time.Time } -func (f FeatureStatus) ToAPI() (apiv1.FeatureStatus, error) { +func (f FeatureStatus) ToAPI() apiv1.FeatureStatus { return apiv1.FeatureStatus{ Enabled: f.Enabled, Message: f.Message, Version: f.Version, UpdatedAt: f.UpdatedAt, - }, nil + } } -func FeatureStatusFromAPI(apiFS apiv1.FeatureStatus) (FeatureStatus, error) { +func FeatureStatusFromAPI(apiFS apiv1.FeatureStatus) FeatureStatus { return FeatureStatus{ Enabled: apiFS.Enabled, Message: apiFS.Message, Version: apiFS.Version, UpdatedAt: apiFS.UpdatedAt, - }, nil + } } diff --git a/src/k8s/pkg/k8sd/types/feature_status_test.go b/src/k8s/pkg/k8sd/types/feature_status_test.go index 17449491e..ab6d2bc4c 100644 --- a/src/k8s/pkg/k8sd/types/feature_status_test.go +++ b/src/k8s/pkg/k8sd/types/feature_status_test.go @@ -4,8 +4,7 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + . "github.com/onsi/gomega" apiv1 "github.com/canonical/k8s/api/v1" "github.com/canonical/k8s/pkg/k8sd/types" @@ -19,12 +18,12 @@ func TestK8sdFeatureStatusToAPI(t *testing.T) { UpdatedAt: time.Now(), } - apiFS, err := k8sdFS.ToAPI() - require.NoError(t, err) - assert.Equal(t, k8sdFS.Enabled, apiFS.Enabled) - assert.Equal(t, k8sdFS.Message, apiFS.Message) - assert.Equal(t, k8sdFS.Version, apiFS.Version) - assert.Equal(t, k8sdFS.UpdatedAt, apiFS.UpdatedAt) + apiFS := k8sdFS.ToAPI() + g := NewWithT(t) + g.Expect(apiFS.Enabled).To(Equal(k8sdFS.Enabled)) + g.Expect(apiFS.Message).To(Equal(k8sdFS.Message)) + g.Expect(apiFS.Version).To(Equal(k8sdFS.Version)) + g.Expect(apiFS.UpdatedAt).To(Equal(k8sdFS.UpdatedAt)) } func TestAPIFeatureStatusToK8sd(t *testing.T) { @@ -35,10 +34,10 @@ func TestAPIFeatureStatusToK8sd(t *testing.T) { UpdatedAt: time.Now(), } - k8sdFS, err := types.FeatureStatusFromAPI(apiFS) - require.NoError(t, err) - assert.Equal(t, apiFS.Enabled, k8sdFS.Enabled) - assert.Equal(t, apiFS.Message, k8sdFS.Message) - assert.Equal(t, apiFS.Version, k8sdFS.Version) - assert.Equal(t, apiFS.UpdatedAt, k8sdFS.UpdatedAt) + k8sdFS := types.FeatureStatusFromAPI(apiFS) + g := NewWithT(t) + g.Expect(k8sdFS.Enabled).To(Equal(apiFS.Enabled)) + g.Expect(k8sdFS.Message).To(Equal(apiFS.Message)) + g.Expect(k8sdFS.Version).To(Equal(apiFS.Version)) + g.Expect(k8sdFS.UpdatedAt).To(Equal(apiFS.UpdatedAt)) } From 66f9583f99b7ec0885ce4e9363e25fe5ead3248f Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 14:52:01 +0400 Subject: [PATCH 10/19] Update output format --- src/k8s/api/v1/types.go | 4 +++- src/k8s/api/v1/types_test.go | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index 5d2212936..f38d14823 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -117,11 +117,13 @@ func (c ClusterStatus) String() string { // Control Plane Nodes result.WriteString(fmt.Sprintf("%-*s ", maxLen, "control plane nodes:")) if len(c.Members) > 0 { + members := make([]string, 0, len(c.Members)) for _, m := range c.Members { if m.ClusterRole == ClusterRoleControlPlane { - result.WriteString(fmt.Sprintf("%s (%s) ", m.Address, m.DatastoreRole)) + members = append(members, fmt.Sprintf("%s (%s)", m.Address, m.DatastoreRole)) } } + result.WriteString(strings.Join(members, ", ")) } else { result.WriteString("none") } diff --git a/src/k8s/api/v1/types_test.go b/src/k8s/api/v1/types_test.go index c7790486e..6a66761d7 100644 --- a/src/k8s/api/v1/types_test.go +++ b/src/k8s/api/v1/types_test.go @@ -1,6 +1,7 @@ package apiv1_test import ( + "fmt" "testing" apiv1 "github.com/canonical/k8s/api/v1" @@ -77,7 +78,7 @@ func TestString(t *testing.T) { Gateway: apiv1.FeatureStatus{Message: "enabled"}, }, expectedOutput: `cluster status: ready -control plane nodes: 192.168.0.1 (voter) 192.168.0.2 (voter) 192.168.0.3 (stand-by) +control plane nodes: 192.168.0.1 (voter), 192.168.0.2 (voter), 192.168.0.3 (stand-by) high availability: no datastore: k8s-dqlite network: enabled @@ -100,7 +101,7 @@ gateway enabled DNS: apiv1.FeatureStatus{Message: "enabled at 192.168.0.10"}, }, expectedOutput: `cluster status: ready -control plane nodes: 192.168.0.1 (voter) +control plane nodes: 192.168.0.1 (voter) high availability: no datastore: external network: enabled @@ -136,6 +137,7 @@ gateway disabled for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) + fmt.Println(tc.clusterStatus.String()) g.Expect(tc.clusterStatus.String()).To(Equal(tc.expectedOutput)) }) } From fba52bc49e6ee0cfed5995584fdc1cab0cdc152f Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Tue, 23 Jul 2024 19:18:36 +0400 Subject: [PATCH 11/19] Address comments --- src/k8s/api/v1/types.go | 44 ++---- src/k8s/api/v1/types_test.go | 2 - src/k8s/pkg/k8sd/controllers/feature.go | 4 +- src/k8s/pkg/k8sd/database/feature_status.go | 11 +- .../pkg/k8sd/database/feature_status_test.go | 130 ++++++++++-------- 5 files changed, 90 insertions(+), 101 deletions(-) diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index f38d14823..accc0ea0f 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -53,7 +53,7 @@ type FeatureStatus struct { UpdatedAt time.Time } -func (f FeatureStatus) GetMessage() string { +func (f FeatureStatus) String() string { if f.Message != "" { return f.Message } @@ -103,25 +103,20 @@ func (c ClusterStatus) HaClusterFormed() bool { func (c ClusterStatus) String() string { result := strings.Builder{} - // longer than the longest key (eye-balled), make the output left aligned - maxLen := 25 - // Status if c.Ready { - result.WriteString(fmt.Sprintf("%-*s %s", maxLen, "cluster status:", "ready")) + result.WriteString(fmt.Sprintf("%-25s %s", "cluster status:", "ready")) } else { - result.WriteString(fmt.Sprintf("%-*s %s", maxLen, "cluster status:", "not ready")) + result.WriteString(fmt.Sprintf("%-25s %s", "cluster status:", "not ready")) } result.WriteString("\n") // Control Plane Nodes - result.WriteString(fmt.Sprintf("%-*s ", maxLen, "control plane nodes:")) + result.WriteString(fmt.Sprintf("%-25s ", "control plane nodes:")) if len(c.Members) > 0 { members := make([]string, 0, len(c.Members)) for _, m := range c.Members { - if m.ClusterRole == ClusterRoleControlPlane { - members = append(members, fmt.Sprintf("%s (%s)", m.Address, m.DatastoreRole)) - } + members = append(members, fmt.Sprintf("%s (%s)", m.Address, m.DatastoreRole)) } result.WriteString(strings.Join(members, ", ")) } else { @@ -130,7 +125,7 @@ func (c ClusterStatus) String() string { result.WriteString("\n") // High availability - result.WriteString(fmt.Sprintf("%-*s ", maxLen, "high availability:")) + result.WriteString(fmt.Sprintf("%-25s ", "high availability:")) if c.HaClusterFormed() { result.WriteString("yes") } else { @@ -141,28 +136,17 @@ func (c ClusterStatus) String() string { // Datastore // TODO: how to understand if the ds is running or not? if c.Datastore.Type != "" { - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "datastore:", c.Datastore.Type)) + result.WriteString(fmt.Sprintf("%-25s %s\n", "datastore:", c.Datastore.Type)) } else { - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "datastore:", "disabled")) + result.WriteString(fmt.Sprintf("%-25s %s\n", "datastore:", "disabled")) } - // Network - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "network:", c.Network.GetMessage())) - - // DNS - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "dns:", c.DNS.GetMessage())) - - // Ingress - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "ingress:", c.Ingress.GetMessage())) - - // Load Balancer - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "load-balancer:", c.LoadBalancer.GetMessage())) - - // Local Storage - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "local-storage:", c.LocalStorage.GetMessage())) - - // Gateway - result.WriteString(fmt.Sprintf("%-*s %s\n", maxLen, "gateway", c.Gateway.GetMessage())) + result.WriteString(fmt.Sprintf("%-25s %s\n", "network:", c.Network)) + result.WriteString(fmt.Sprintf("%-25s %s\n", "dns:", c.DNS)) + result.WriteString(fmt.Sprintf("%-25s %s\n", "ingress:", c.Ingress)) + result.WriteString(fmt.Sprintf("%-25s %s\n", "load-balancer:", c.LoadBalancer)) + result.WriteString(fmt.Sprintf("%-25s %s\n", "local-storage:", c.LocalStorage)) + result.WriteString(fmt.Sprintf("%-25s %s\n", "gateway", c.Gateway)) return result.String() } diff --git a/src/k8s/api/v1/types_test.go b/src/k8s/api/v1/types_test.go index 6a66761d7..a9a815b0f 100644 --- a/src/k8s/api/v1/types_test.go +++ b/src/k8s/api/v1/types_test.go @@ -1,7 +1,6 @@ package apiv1_test import ( - "fmt" "testing" apiv1 "github.com/canonical/k8s/api/v1" @@ -137,7 +136,6 @@ gateway disabled for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - fmt.Println(tc.clusterStatus.String()) g.Expect(tc.clusterStatus.String()).To(Equal(tc.expectedOutput)) }) } diff --git a/src/k8s/pkg/k8sd/controllers/feature.go b/src/k8s/pkg/k8sd/controllers/feature.go index 98073b59a..b835411bc 100644 --- a/src/k8s/pkg/k8sd/controllers/feature.go +++ b/src/k8s/pkg/k8sd/controllers/feature.go @@ -134,9 +134,7 @@ func (c *FeatureController) reconcile( featureStatus, err := apply(cfg) if err != nil { return fmt.Errorf("failed to apply configuration: %w", err) - } - - if err := updateFeatureStatus(ctx, featureStatus); err != nil { + } else if err := updateFeatureStatus(ctx, featureStatus); err != nil { return fmt.Errorf("failed to update feature status: %w", err) } diff --git a/src/k8s/pkg/k8sd/database/feature_status.go b/src/k8s/pkg/k8sd/database/feature_status.go index b6e7b1f7a..c31ac6322 100644 --- a/src/k8s/pkg/k8sd/database/feature_status.go +++ b/src/k8s/pkg/k8sd/database/feature_status.go @@ -52,18 +52,17 @@ func GetFeatureStatuses(ctx context.Context, tx *sql.Tx) (map[string]types.Featu for rows.Next() { var ( - name string - ts string + name string + ts string + status types.FeatureStatus ) - status := types.FeatureStatus{} if err := rows.Scan(&name, &status.Message, &status.Version, &ts, &status.Enabled); err != nil { return nil, fmt.Errorf("failed to scan row: %w", err) } - status.UpdatedAt, err = time.Parse(time.RFC3339, ts) - if err != nil { - log.L().Error(err, "failed to parse time", "original", ts) + if status.UpdatedAt, err = time.Parse(time.RFC3339, ts); err != nil { + log.FromContext(ctx).Error(err, "failed to parse time", "original", ts) } result[name] = status diff --git a/src/k8s/pkg/k8sd/database/feature_status_test.go b/src/k8s/pkg/k8sd/database/feature_status_test.go index cedf1f93c..3bb635d28 100644 --- a/src/k8s/pkg/k8sd/database/feature_status_test.go +++ b/src/k8s/pkg/k8sd/database/feature_status_test.go @@ -15,13 +15,7 @@ import ( func TestFeatureStatus(t *testing.T) { WithDB(t, func(ctx context.Context, db DB) { _ = db.Transaction(ctx, func(ctx context.Context, tx *sql.Tx) error { - g := NewWithT(t) t0, _ := time.Parse(time.RFC3339, time.Now().Format(time.RFC3339)) - // initial get should return nothing - ss, err := database.GetFeatureStatuses(ctx, tx) - g.Expect(err).To(BeNil()) - g.Expect(len(ss)).To(Equal(0)) - networkS := types.FeatureStatus{ Enabled: true, Message: "enabled", @@ -34,28 +28,6 @@ func TestFeatureStatus(t *testing.T) { Version: "4.5.6", UpdatedAt: t0, } - // setting new values - err = database.SetFeatureStatus(ctx, tx, "network", networkS) - g.Expect(err).To(BeNil()) - err = database.SetFeatureStatus(ctx, tx, "dns", dnsS) - g.Expect(err).To(BeNil()) - - // getting new values - ss, err = database.GetFeatureStatuses(ctx, tx) - g.Expect(err).To(BeNil()) - g.Expect(len(ss)).To(Equal(2)) - - g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) - g.Expect(ss["network"].Message).To(Equal(networkS.Message)) - g.Expect(ss["network"].Version).To(Equal(networkS.Version)) - g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) - - g.Expect(ss["dns"].Enabled).To(Equal(dnsS.Enabled)) - g.Expect(ss["dns"].Message).To(Equal(dnsS.Message)) - g.Expect(ss["dns"].Version).To(Equal(dnsS.Version)) - g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS.UpdatedAt)) - - // updating old values and adding new ones dnsS2 := types.FeatureStatus{ Enabled: true, Message: "enabled at 10.0.0.2", @@ -68,38 +40,76 @@ func TestFeatureStatus(t *testing.T) { Version: "10.20.30", UpdatedAt: t0, } - // setting the old value for network again - err = database.SetFeatureStatus(ctx, tx, "network", networkS) - g.Expect(err).To(BeNil()) - // updating dns with new value - err = database.SetFeatureStatus(ctx, tx, "dns", dnsS2) - g.Expect(err).To(BeNil()) - // adding new status - err = database.SetFeatureStatus(ctx, tx, "gateway", gatewayS) - g.Expect(err).To(BeNil()) - - // checking the new values - ss, err = database.GetFeatureStatuses(ctx, tx) - g.Expect(err).To(BeNil()) - g.Expect(len(ss)).To(Equal(3)) - - // network stayed the same - g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) - g.Expect(ss["network"].Message).To(Equal(networkS.Message)) - g.Expect(ss["network"].Version).To(Equal(networkS.Version)) - g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) - - // dns is updated - g.Expect(ss["dns"].Enabled).To(Equal(dnsS2.Enabled)) - g.Expect(ss["dns"].Message).To(Equal(dnsS2.Message)) - g.Expect(ss["dns"].Version).To(Equal(dnsS2.Version)) - g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS2.UpdatedAt)) - - // gateway is added - g.Expect(ss["gateway"].Enabled).To(Equal(gatewayS.Enabled)) - g.Expect(ss["gateway"].Message).To(Equal(gatewayS.Message)) - g.Expect(ss["gateway"].Version).To(Equal(gatewayS.Version)) - g.Expect(ss["gateway"].UpdatedAt).To(Equal(gatewayS.UpdatedAt)) + + t.Run("ReturnNothingInitially", func(t *testing.T) { + g := NewWithT(t) + ss, err := database.GetFeatureStatuses(ctx, tx) + g.Expect(err).To(BeNil()) + g.Expect(len(ss)).To(Equal(0)) + + }) + + t.Run("SettingNewStatus", func(t *testing.T) { + g := NewWithT(t) + + err := database.SetFeatureStatus(ctx, tx, "network", networkS) + g.Expect(err).To(BeNil()) + err = database.SetFeatureStatus(ctx, tx, "dns", dnsS) + g.Expect(err).To(BeNil()) + + ss, err := database.GetFeatureStatuses(ctx, tx) + g.Expect(err).To(BeNil()) + g.Expect(len(ss)).To(Equal(2)) + + g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) + g.Expect(ss["network"].Message).To(Equal(networkS.Message)) + g.Expect(ss["network"].Version).To(Equal(networkS.Version)) + g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) + + g.Expect(ss["dns"].Enabled).To(Equal(dnsS.Enabled)) + g.Expect(ss["dns"].Message).To(Equal(dnsS.Message)) + g.Expect(ss["dns"].Version).To(Equal(dnsS.Version)) + g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS.UpdatedAt)) + + }) + t.Run("UpdatingStatus", func(t *testing.T) { + g := NewWithT(t) + + err := database.SetFeatureStatus(ctx, tx, "network", networkS) + g.Expect(err).To(BeNil()) + err = database.SetFeatureStatus(ctx, tx, "dns", dnsS) + g.Expect(err).To(BeNil()) + + // set and update + err = database.SetFeatureStatus(ctx, tx, "network", networkS) + g.Expect(err).To(BeNil()) + err = database.SetFeatureStatus(ctx, tx, "dns", dnsS2) + g.Expect(err).To(BeNil()) + err = database.SetFeatureStatus(ctx, tx, "gateway", gatewayS) + g.Expect(err).To(BeNil()) + + ss, err := database.GetFeatureStatuses(ctx, tx) + g.Expect(err).To(BeNil()) + g.Expect(len(ss)).To(Equal(3)) + + // network stayed the same + g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) + g.Expect(ss["network"].Message).To(Equal(networkS.Message)) + g.Expect(ss["network"].Version).To(Equal(networkS.Version)) + g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) + + // dns is updated + g.Expect(ss["dns"].Enabled).To(Equal(dnsS2.Enabled)) + g.Expect(ss["dns"].Message).To(Equal(dnsS2.Message)) + g.Expect(ss["dns"].Version).To(Equal(dnsS2.Version)) + g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS2.UpdatedAt)) + + // gateway is added + g.Expect(ss["gateway"].Enabled).To(Equal(gatewayS.Enabled)) + g.Expect(ss["gateway"].Message).To(Equal(gatewayS.Message)) + g.Expect(ss["gateway"].Version).To(Equal(gatewayS.Version)) + g.Expect(ss["gateway"].UpdatedAt).To(Equal(gatewayS.UpdatedAt)) + }) return nil }) From ae4d54518c84f7857f9656f9b267657a51f2f3b8 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Wed, 24 Jul 2024 10:33:19 +0400 Subject: [PATCH 12/19] Add smoke test, Address comments and issues --- .gitignore | 3 +- src/k8s/api/v1/types.go | 2 +- src/k8s/api/v1/types_test.go | 9 +-- .../pkg/k8sd/database/feature_status_test.go | 68 +++++++++---------- tests/integration/tests/test_smoke.py | 21 +++++- tests/integration/tests/test_util/util.py | 9 ++- 6 files changed, 66 insertions(+), 46 deletions(-) diff --git a/.gitignore b/.gitignore index 3c2d828af..976bbd3fa 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /stage/ /prime/ .vscode/ +env/ **.snap @@ -21,4 +22,4 @@ k8s_*.txt /docs/tools/.sphinx/warnings.txt /docs/tools/.sphinx/.wordlist.dic /docs/tools/.sphinx/.doctrees/ -/docs/tools/.sphinx/node_modules \ No newline at end of file +/docs/tools/.sphinx/node_modules diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index accc0ea0f..4aef3ea04 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -146,7 +146,7 @@ func (c ClusterStatus) String() string { result.WriteString(fmt.Sprintf("%-25s %s\n", "ingress:", c.Ingress)) result.WriteString(fmt.Sprintf("%-25s %s\n", "load-balancer:", c.LoadBalancer)) result.WriteString(fmt.Sprintf("%-25s %s\n", "local-storage:", c.LocalStorage)) - result.WriteString(fmt.Sprintf("%-25s %s\n", "gateway", c.Gateway)) + result.WriteString(fmt.Sprintf("%-25s %s", "gateway", c.Gateway)) return result.String() } diff --git a/src/k8s/api/v1/types_test.go b/src/k8s/api/v1/types_test.go index a9a815b0f..8a5c02cfb 100644 --- a/src/k8s/api/v1/types_test.go +++ b/src/k8s/api/v1/types_test.go @@ -85,8 +85,7 @@ dns: enabled at 192.168.0.10 ingress: enabled load-balancer: enabled, L2 mode local-storage: enabled at /var/snap/k8s/common/rawfile-storage -gateway enabled -`, +gateway enabled`, }, { name: "External Datastore", @@ -108,8 +107,7 @@ dns: enabled at 192.168.0.10 ingress: disabled load-balancer: disabled local-storage: disabled -gateway disabled -`, +gateway disabled`, }, { name: "Cluster not ready, HA not formed, no nodes", @@ -128,8 +126,7 @@ dns: disabled ingress: disabled load-balancer: disabled local-storage: disabled -gateway disabled -`, +gateway disabled`, }, } diff --git a/src/k8s/pkg/k8sd/database/feature_status_test.go b/src/k8s/pkg/k8sd/database/feature_status_test.go index 3bb635d28..f18df401e 100644 --- a/src/k8s/pkg/k8sd/database/feature_status_test.go +++ b/src/k8s/pkg/k8sd/database/feature_status_test.go @@ -16,25 +16,25 @@ func TestFeatureStatus(t *testing.T) { WithDB(t, func(ctx context.Context, db DB) { _ = db.Transaction(ctx, func(ctx context.Context, tx *sql.Tx) error { t0, _ := time.Parse(time.RFC3339, time.Now().Format(time.RFC3339)) - networkS := types.FeatureStatus{ + networkStatus := types.FeatureStatus{ Enabled: true, Message: "enabled", Version: "1.2.3", UpdatedAt: t0, } - dnsS := types.FeatureStatus{ + dnsStatus := types.FeatureStatus{ Enabled: true, Message: "enabled at 10.0.0.1", Version: "4.5.6", UpdatedAt: t0, } - dnsS2 := types.FeatureStatus{ + dnsStatus2 := types.FeatureStatus{ Enabled: true, Message: "enabled at 10.0.0.2", Version: "4.5.7", UpdatedAt: t0, } - gatewayS := types.FeatureStatus{ + gatewayStatus := types.FeatureStatus{ Enabled: true, Message: "disabled", Version: "10.20.30", @@ -45,70 +45,70 @@ func TestFeatureStatus(t *testing.T) { g := NewWithT(t) ss, err := database.GetFeatureStatuses(ctx, tx) g.Expect(err).To(BeNil()) - g.Expect(len(ss)).To(Equal(0)) + g.Expect(ss).To(BeEmpty()) }) t.Run("SettingNewStatus", func(t *testing.T) { g := NewWithT(t) - err := database.SetFeatureStatus(ctx, tx, "network", networkS) + err := database.SetFeatureStatus(ctx, tx, "network", networkStatus) g.Expect(err).To(BeNil()) - err = database.SetFeatureStatus(ctx, tx, "dns", dnsS) + err = database.SetFeatureStatus(ctx, tx, "dns", dnsStatus) g.Expect(err).To(BeNil()) ss, err := database.GetFeatureStatuses(ctx, tx) g.Expect(err).To(BeNil()) - g.Expect(len(ss)).To(Equal(2)) + g.Expect(ss).To(HaveLen(2)) - g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) - g.Expect(ss["network"].Message).To(Equal(networkS.Message)) - g.Expect(ss["network"].Version).To(Equal(networkS.Version)) - g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) + g.Expect(ss["network"].Enabled).To(Equal(networkStatus.Enabled)) + g.Expect(ss["network"].Message).To(Equal(networkStatus.Message)) + g.Expect(ss["network"].Version).To(Equal(networkStatus.Version)) + g.Expect(ss["network"].UpdatedAt).To(Equal(networkStatus.UpdatedAt)) - g.Expect(ss["dns"].Enabled).To(Equal(dnsS.Enabled)) - g.Expect(ss["dns"].Message).To(Equal(dnsS.Message)) - g.Expect(ss["dns"].Version).To(Equal(dnsS.Version)) - g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS.UpdatedAt)) + g.Expect(ss["dns"].Enabled).To(Equal(dnsStatus.Enabled)) + g.Expect(ss["dns"].Message).To(Equal(dnsStatus.Message)) + g.Expect(ss["dns"].Version).To(Equal(dnsStatus.Version)) + g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsStatus.UpdatedAt)) }) t.Run("UpdatingStatus", func(t *testing.T) { g := NewWithT(t) - err := database.SetFeatureStatus(ctx, tx, "network", networkS) + err := database.SetFeatureStatus(ctx, tx, "network", networkStatus) g.Expect(err).To(BeNil()) - err = database.SetFeatureStatus(ctx, tx, "dns", dnsS) + err = database.SetFeatureStatus(ctx, tx, "dns", dnsStatus) g.Expect(err).To(BeNil()) // set and update - err = database.SetFeatureStatus(ctx, tx, "network", networkS) + err = database.SetFeatureStatus(ctx, tx, "network", networkStatus) g.Expect(err).To(BeNil()) - err = database.SetFeatureStatus(ctx, tx, "dns", dnsS2) + err = database.SetFeatureStatus(ctx, tx, "dns", dnsStatus2) g.Expect(err).To(BeNil()) - err = database.SetFeatureStatus(ctx, tx, "gateway", gatewayS) + err = database.SetFeatureStatus(ctx, tx, "gateway", gatewayStatus) g.Expect(err).To(BeNil()) ss, err := database.GetFeatureStatuses(ctx, tx) g.Expect(err).To(BeNil()) - g.Expect(len(ss)).To(Equal(3)) + g.Expect(ss).To(HaveLen(3)) // network stayed the same - g.Expect(ss["network"].Enabled).To(Equal(networkS.Enabled)) - g.Expect(ss["network"].Message).To(Equal(networkS.Message)) - g.Expect(ss["network"].Version).To(Equal(networkS.Version)) - g.Expect(ss["network"].UpdatedAt).To(Equal(networkS.UpdatedAt)) + g.Expect(ss["network"].Enabled).To(Equal(networkStatus.Enabled)) + g.Expect(ss["network"].Message).To(Equal(networkStatus.Message)) + g.Expect(ss["network"].Version).To(Equal(networkStatus.Version)) + g.Expect(ss["network"].UpdatedAt).To(Equal(networkStatus.UpdatedAt)) // dns is updated - g.Expect(ss["dns"].Enabled).To(Equal(dnsS2.Enabled)) - g.Expect(ss["dns"].Message).To(Equal(dnsS2.Message)) - g.Expect(ss["dns"].Version).To(Equal(dnsS2.Version)) - g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsS2.UpdatedAt)) + g.Expect(ss["dns"].Enabled).To(Equal(dnsStatus2.Enabled)) + g.Expect(ss["dns"].Message).To(Equal(dnsStatus2.Message)) + g.Expect(ss["dns"].Version).To(Equal(dnsStatus2.Version)) + g.Expect(ss["dns"].UpdatedAt).To(Equal(dnsStatus2.UpdatedAt)) // gateway is added - g.Expect(ss["gateway"].Enabled).To(Equal(gatewayS.Enabled)) - g.Expect(ss["gateway"].Message).To(Equal(gatewayS.Message)) - g.Expect(ss["gateway"].Version).To(Equal(gatewayS.Version)) - g.Expect(ss["gateway"].UpdatedAt).To(Equal(gatewayS.UpdatedAt)) + g.Expect(ss["gateway"].Enabled).To(Equal(gatewayStatus.Enabled)) + g.Expect(ss["gateway"].Message).To(Equal(gatewayStatus.Message)) + g.Expect(ss["gateway"].Version).To(Equal(gatewayStatus.Version)) + g.Expect(ss["gateway"].UpdatedAt).To(Equal(gatewayStatus.UpdatedAt)) }) return nil diff --git a/tests/integration/tests/test_smoke.py b/tests/integration/tests/test_smoke.py index f60d8cef9..26611608c 100644 --- a/tests/integration/tests/test_smoke.py +++ b/tests/integration/tests/test_smoke.py @@ -2,6 +2,7 @@ # Copyright 2024 Canonical, Ltd. # import json +import re import logging from typing import List @@ -23,7 +24,7 @@ def test_smoke(instances: List[harness.Instance]): ) instance.exec(["k8s", "bootstrap", "--file", bootstrap_smoke_config_path]) - util.wait_until_k8s_ready(instance, [instance]) + util.wait_until_k8s_ready(instance, [instance], retries=120, delay_s=10) # Verify the functionality of the k8s config command during the smoke test. # It would be excessive to deploy a cluster solely for this purpose. @@ -92,3 +93,21 @@ def test_smoke(instances: List[harness.Instance]): assert ( metadata.get("token") is not None ), "Token not found in the generate-join-token response." + + # Verify output of the k8s status + result = instance.exec(["k8s", "status"], capture_output=True) + patterns = [ + r"cluster status:\s*ready", + r"control plane nodes:\s*(\d{1,3}(?:\.\d{1,3}){3}:\d{1,5})\s\(voter\)", + r"high availability:\s*no", + r"datastore:\s*k8s-dqlite", + r"network:\s*enabled", + r"dns:\s*enabled at (\d{1,3}(?:\.\d{1,3}){3})", + r"ingress:\s*enabled", + r"load-balancer:\s*enabled, Unknown mode", + r"local-storage:\s*enabled at /var/snap/k8s/common/rawfile-storage", + r"gateway\s*enabled", + ] + for line, pattern in zip(result.stdout.decode().split("\n"), patterns): + assert re.search(pattern, line), "%s did not match %s".format(line, pattern) + diff --git a/tests/integration/tests/test_util/util.py b/tests/integration/tests/test_util/util.py index c9a62b452..68a614c52 100644 --- a/tests/integration/tests/test_util/util.py +++ b/tests/integration/tests/test_util/util.py @@ -136,14 +136,17 @@ def setup_k8s_snap(instance: harness.Instance, snap_path: Path): instance.exec(["/snap/k8s/current/k8s/hack/init.sh"], stdout=subprocess.DEVNULL) -# Validates that the K8s node is in Ready state. def wait_until_k8s_ready( - control_node: harness.Instance, instances: List[harness.Instance] + control_node: harness.Instance, instances: List[harness.Instance], + retries: int = 15, delay_s: int = 5, ): + """ + Validates that the K8s node is in Ready state. + """ for instance in instances: host = hostname(instance) result = ( - stubbornly(retries=15, delay_s=5) + stubbornly(retries=retries, delay_s=delay_s) .on(control_node) .until(lambda p: " Ready" in p.stdout.decode()) .exec(["k8s", "kubectl", "get", "node", host, "--no-headers"]) From 9063cac391f63350ffd0f7293e267cd5be55859c Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Wed, 24 Jul 2024 10:55:04 +0400 Subject: [PATCH 13/19] Update docs --- docs/src/_parts/commands/k8s_status.md | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/src/_parts/commands/k8s_status.md b/docs/src/_parts/commands/k8s_status.md index c3e2789a5..ab2b9ea98 100644 --- a/docs/src/_parts/commands/k8s_status.md +++ b/docs/src/_parts/commands/k8s_status.md @@ -2,6 +2,34 @@ Retrieve the current status of the cluster +### Synopsis + +Retrieve the current status of the cluster. Also, The status for each +core feature is described with the following fields: +- **Enabled**: Specifies whether the feature is successfully deployed (does not guarantee that the feature is working as expected). +- **Message**: Describes the status in a human readable form. This field is only +meant to be informative and should not be programmatically parsed in any way. +- **Version**: Version of the feature. +- **UpdatedAt**: Timestamp of the lastest update to the feature status. + +Can be used with `--output-format=plain` +to show a compact, human readable output, or `--output-format=json` or `yaml` flag to show +more information. + +Example with `--output-format=plain` would be: +```text +cluster status: ready +control plane nodes: 10.97.72.156:6400 (voter) +high availability: no +datastore: k8s-dqlite +network: enabled +dns: enabled at 10.152.183.26 +ingress: enabled +load-balancer: enabled, L2 mode +local-storage: enabled at /var/snap/k8s/common/rawfile-storage +gateway enabled +``` + ``` k8s status [flags] ``` From 73b7dac9474a73d31cbc940150933c6b6f6d98ff Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Wed, 24 Jul 2024 11:20:36 +0400 Subject: [PATCH 14/19] Fix Enabled field inconsistency --- src/k8s/pkg/k8sd/features/calico/network.go | 4 ++++ src/k8s/pkg/k8sd/features/cilium/gateway.go | 4 ++++ src/k8s/pkg/k8sd/features/cilium/ingress.go | 2 ++ src/k8s/pkg/k8sd/features/cilium/loadbalancer.go | 1 + src/k8s/pkg/k8sd/features/cilium/network.go | 7 +++++++ src/k8s/pkg/k8sd/features/contour/gateway.go | 3 +++ src/k8s/pkg/k8sd/features/contour/ingress.go | 6 ++++++ src/k8s/pkg/k8sd/features/coredns/coredns.go | 9 +++++++-- src/k8s/pkg/k8sd/features/localpv/localpv.go | 1 + src/k8s/pkg/k8sd/features/metallb/loadbalancer.go | 1 + .../pkg/k8sd/features/metrics-server/metrics_server.go | 1 + 11 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/k8s/pkg/k8sd/features/calico/network.go b/src/k8s/pkg/k8sd/features/calico/network.go index 0657711e3..0cc8a4509 100644 --- a/src/k8s/pkg/k8sd/features/calico/network.go +++ b/src/k8s/pkg/k8sd/features/calico/network.go @@ -46,6 +46,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota if err != nil { cfgErr := fmt.Errorf("failed to parse annotations: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, cfgErr) + status.Enabled = false return status, cfgErr } @@ -54,6 +55,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota if err != nil { cidrErr := fmt.Errorf("invalid pod cidr: %v", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, cidrErr) + status.Enabled = false return status, cidrErr } if ipv4PodCIDR != "" { @@ -76,6 +78,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota if err != nil { cidrErr := fmt.Errorf("invalid service cidr: %v", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, cidrErr) + status.Enabled = false return status, cidrErr } if ipv4ServiceCIDR != "" { @@ -120,6 +123,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota if _, err := m.Apply(ctx, chartCalico, helm.StatePresent, values); err != nil { enableErr := fmt.Errorf("failed to enable network: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } diff --git a/src/k8s/pkg/k8sd/features/cilium/gateway.go b/src/k8s/pkg/k8sd/features/cilium/gateway.go index a263165c9..6c0a06a1a 100644 --- a/src/k8s/pkg/k8sd/features/cilium/gateway.go +++ b/src/k8s/pkg/k8sd/features/cilium/gateway.go @@ -33,6 +33,7 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne if gateway.GetEnabled() { enableErr := fmt.Errorf("failed to install Gateway API CRDs: %w", err) status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } else { disableErr := fmt.Errorf("failed to delete Gateway API CRDs: %w", err) @@ -46,6 +47,7 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne if gateway.GetEnabled() { enableErr := fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } else { disableErr := fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) @@ -59,6 +61,7 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne if gateway.GetEnabled() { enableErr := fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } else { disableErr := fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) @@ -86,6 +89,7 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne if err := rolloutRestartCilium(ctx, snap, 3); err != nil { resErr := fmt.Errorf("failed to rollout restart cilium to apply Gateway API: %w", err) status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, resErr) + status.Enabled = false return status, resErr } diff --git a/src/k8s/pkg/k8sd/features/cilium/ingress.go b/src/k8s/pkg/k8sd/features/cilium/ingress.go index 50106a1c2..868899324 100644 --- a/src/k8s/pkg/k8sd/features/cilium/ingress.go +++ b/src/k8s/pkg/k8sd/features/cilium/ingress.go @@ -56,6 +56,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, ne if network.GetEnabled() { enableErr := fmt.Errorf("failed to enable ingress: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } else { disableErr := fmt.Errorf("failed to disable ingress: %w", err) @@ -84,6 +85,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, ne if err := rolloutRestartCilium(ctx, snap, 3); err != nil { restartErr := fmt.Errorf("failed to rollout restart cilium to apply ingress: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, restartErr) + status.Enabled = false return status, restartErr } diff --git a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go index fb0e36c40..b7dc05720 100644 --- a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go @@ -44,6 +44,7 @@ func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.L if err := enableLoadBalancer(ctx, snap, loadbalancer, network); err != nil { enableErr := fmt.Errorf("failed to enable LoadBalancer: %w", err) status.Message = fmt.Sprintf(lbDeployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } diff --git a/src/k8s/pkg/k8sd/features/cilium/network.go b/src/k8s/pkg/k8sd/features/cilium/network.go index 857ca25a4..8268d5251 100644 --- a/src/k8s/pkg/k8sd/features/cilium/network.go +++ b/src/k8s/pkg/k8sd/features/cilium/network.go @@ -47,6 +47,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if err != nil { cidrErr := fmt.Errorf("invalid kube-proxy --cluster-cidr value: %v", err) status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, cidrErr) + status.Enabled = false return status, cidrErr } @@ -94,6 +95,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if err != nil { mntErr := fmt.Errorf("failed to get bpf mount path: %w", err) status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, mntErr) + status.Enabled = false return status, mntErr } @@ -101,6 +103,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if err != nil { cgrpErr := fmt.Errorf("failed to get cgroup2 mount path: %w", err) status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, cgrpErr) + status.Enabled = false return status, cgrpErr } @@ -121,6 +124,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if err != nil { mntErr := fmt.Errorf("failed to get mount propagation for %s: %w", p, err) status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, mntErr) + status.Enabled = false return status, mntErr } if p == "private" { @@ -131,11 +135,13 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if onLXD { lxdErr := fmt.Errorf("/sys is not a shared mount on the LXD container, this might be resolved by updating LXD on the host to version 5.0.2 or newer") status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, lxdErr) + status.Enabled = false return status, lxdErr } sysErr := fmt.Errorf("/sys is not a shared mount") status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, sysErr) + status.Enabled = false return status, sysErr } } @@ -143,6 +149,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if _, err := m.Apply(ctx, chartCilium, helm.StatePresent, values); err != nil { enableErr := fmt.Errorf("failed to enable network: %w", err) status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } diff --git a/src/k8s/pkg/k8sd/features/contour/gateway.go b/src/k8s/pkg/k8sd/features/contour/gateway.go index df86de1da..cf66b8390 100644 --- a/src/k8s/pkg/k8sd/features/contour/gateway.go +++ b/src/k8s/pkg/k8sd/features/contour/gateway.go @@ -46,12 +46,14 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne if err := applyCommonContourCRDS(ctx, snap, true); err != nil { crdErr := fmt.Errorf("failed to apply common contour CRDS: %w", err) status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, crdErr) + status.Enabled = false return status, crdErr } if err := waitForRequiredContourCommonCRDs(ctx, snap); err != nil { waitErr := fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, waitErr) + status.Enabled = false return status, waitErr } @@ -73,6 +75,7 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne if _, err := m.Apply(ctx, chartGateway, helm.StatePresent, values); err != nil { installErr := fmt.Errorf("failed to install the contour gateway chart: %w", err) status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, installErr) + status.Enabled = false return status, installErr } diff --git a/src/k8s/pkg/k8sd/features/contour/ingress.go b/src/k8s/pkg/k8sd/features/contour/ingress.go index d9b43c3d1..b501cebb7 100644 --- a/src/k8s/pkg/k8sd/features/contour/ingress.go +++ b/src/k8s/pkg/k8sd/features/contour/ingress.go @@ -47,12 +47,14 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ if err := applyCommonContourCRDS(ctx, snap, true); err != nil { crdErr := fmt.Errorf("failed to apply common contour CRDS: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, crdErr) + status.Enabled = false return status, crdErr } if err := waitForRequiredContourCommonCRDs(ctx, snap); err != nil { waitErr := fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, waitErr) + status.Enabled = false return status, waitErr } @@ -92,6 +94,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ if err != nil { enableErr := fmt.Errorf("failed to enable ingress: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } @@ -99,6 +102,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ if err := rolloutRestartContour(ctx, snap, 3); err != nil { resErr := fmt.Errorf("failed to rollout restart contour to apply ingress: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, resErr) + status.Enabled = false return status, resErr } } @@ -113,6 +117,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ if _, err := m.Apply(ctx, chartDefaultTLS, helm.StatePresent, values); err != nil { tlsErr := fmt.Errorf("failed to install the delegation resource for default TLS secret: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, tlsErr) + status.Enabled = false return status, tlsErr } status.Message = enabledMsg @@ -122,6 +127,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ if _, err := m.Apply(ctx, chartDefaultTLS, helm.StateDeleted, nil); err != nil { tlsErr := fmt.Errorf("failed to uninstall the delegation resource for default TLS secret: %w", err) status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, tlsErr) + status.Enabled = false return status, tlsErr } diff --git a/src/k8s/pkg/k8sd/features/coredns/coredns.go b/src/k8s/pkg/k8sd/features/coredns/coredns.go index 2bff221d0..0761a1201 100644 --- a/src/k8s/pkg/k8sd/features/coredns/coredns.go +++ b/src/k8s/pkg/k8sd/features/coredns/coredns.go @@ -26,11 +26,13 @@ const ( // ApplyDNS returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types.Kubelet, _ types.Annotations) (types.FeatureStatus, string, error) { - status := types.FeatureStatus{Version: imageTag} + status := types.FeatureStatus{ + Version: imageTag, + Enabled: dns.GetEnabled(), + } m := snap.HelmClient() if !dns.GetEnabled() { - status.Enabled = false if _, err := m.Apply(ctx, chart, helm.StateDeleted, nil); err != nil { delErr := fmt.Errorf("failed to uninstall coredns: %w", err) status.Message = fmt.Sprintf(deleteFailedMsgTmpl, delErr) @@ -82,6 +84,7 @@ func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types. if _, err := m.Apply(ctx, chart, helm.StatePresent, values); err != nil { applyErr := fmt.Errorf("failed to apply coredns: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, applyErr) + status.Enabled = false return status, "", applyErr } @@ -89,12 +92,14 @@ func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types. if err != nil { clientErr := fmt.Errorf("failed to create kubernetes client: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, clientErr) + status.Enabled = false return status, "", clientErr } dnsIP, err := client.GetServiceClusterIP(ctx, "coredns", "kube-system") if err != nil { retErr := fmt.Errorf("failed to retrieve the coredns service: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, retErr) + status.Enabled = false return status, "", retErr } diff --git a/src/k8s/pkg/k8sd/features/localpv/localpv.go b/src/k8s/pkg/k8sd/features/localpv/localpv.go index fda7eb5f1..6439d2c7d 100644 --- a/src/k8s/pkg/k8sd/features/localpv/localpv.go +++ b/src/k8s/pkg/k8sd/features/localpv/localpv.go @@ -67,6 +67,7 @@ func ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStora if cfg.GetEnabled() { enableErr := fmt.Errorf("failed to install rawfile-csi helm package: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } else { disableErr := fmt.Errorf("failed to delete rawfile-csi helm package: %w", err) diff --git a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go index 5c37feaf0..cb9def6e9 100644 --- a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go @@ -41,6 +41,7 @@ func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.L if err := enableLoadBalancer(ctx, snap, loadbalancer, network); err != nil { enableErr := fmt.Errorf("failed to enable LoadBalancer: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } diff --git a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go index 63040726f..56fc1c764 100644 --- a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go +++ b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go @@ -47,6 +47,7 @@ func ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsSe if cfg.GetEnabled() { enableErr := fmt.Errorf("failed to install metrics server chart: %w", err) status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) + status.Enabled = false return status, enableErr } else { disableErr := fmt.Errorf("failed to delete metrics server chart: %w", err) From 3a6efc5fc0ba76f99b2bb0a46da59bfd05b1aa15 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Wed, 24 Jul 2024 11:59:55 +0400 Subject: [PATCH 15/19] Fix lint and docs --- docs/src/_parts/commands/k8s_status.md | 26 +---------------------- src/k8s/cmd/k8s/k8s_status.go | 1 + tests/integration/tests/test_smoke.py | 5 ++--- tests/integration/tests/test_util/util.py | 6 ++++-- 4 files changed, 8 insertions(+), 30 deletions(-) diff --git a/docs/src/_parts/commands/k8s_status.md b/docs/src/_parts/commands/k8s_status.md index ab2b9ea98..06b9c825b 100644 --- a/docs/src/_parts/commands/k8s_status.md +++ b/docs/src/_parts/commands/k8s_status.md @@ -4,31 +4,7 @@ Retrieve the current status of the cluster ### Synopsis -Retrieve the current status of the cluster. Also, The status for each -core feature is described with the following fields: -- **Enabled**: Specifies whether the feature is successfully deployed (does not guarantee that the feature is working as expected). -- **Message**: Describes the status in a human readable form. This field is only -meant to be informative and should not be programmatically parsed in any way. -- **Version**: Version of the feature. -- **UpdatedAt**: Timestamp of the lastest update to the feature status. - -Can be used with `--output-format=plain` -to show a compact, human readable output, or `--output-format=json` or `yaml` flag to show -more information. - -Example with `--output-format=plain` would be: -```text -cluster status: ready -control plane nodes: 10.97.72.156:6400 (voter) -high availability: no -datastore: k8s-dqlite -network: enabled -dns: enabled at 10.152.183.26 -ingress: enabled -load-balancer: enabled, L2 mode -local-storage: enabled at /var/snap/k8s/common/rawfile-storage -gateway enabled -``` +Retrieve the current status of the cluster as well as deployment status of core features. ``` k8s status [flags] diff --git a/src/k8s/cmd/k8s/k8s_status.go b/src/k8s/cmd/k8s/k8s_status.go index e5ffdec5b..ea6c64f0a 100644 --- a/src/k8s/cmd/k8s/k8s_status.go +++ b/src/k8s/cmd/k8s/k8s_status.go @@ -18,6 +18,7 @@ func newStatusCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ Use: "status", Short: "Retrieve the current status of the cluster", + Long: "Retrieve the current status of the cluster as well as deployment status of core features.", PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat)), Run: func(cmd *cobra.Command, args []string) { if opts.timeout < minTimeout { diff --git a/tests/integration/tests/test_smoke.py b/tests/integration/tests/test_smoke.py index 26611608c..57a79e3b6 100644 --- a/tests/integration/tests/test_smoke.py +++ b/tests/integration/tests/test_smoke.py @@ -2,8 +2,8 @@ # Copyright 2024 Canonical, Ltd. # import json -import re import logging +import re from typing import List import pytest @@ -109,5 +109,4 @@ def test_smoke(instances: List[harness.Instance]): r"gateway\s*enabled", ] for line, pattern in zip(result.stdout.decode().split("\n"), patterns): - assert re.search(pattern, line), "%s did not match %s".format(line, pattern) - + assert re.search(pattern, line) diff --git a/tests/integration/tests/test_util/util.py b/tests/integration/tests/test_util/util.py index 68a614c52..33f4c5087 100644 --- a/tests/integration/tests/test_util/util.py +++ b/tests/integration/tests/test_util/util.py @@ -137,8 +137,10 @@ def setup_k8s_snap(instance: harness.Instance, snap_path: Path): def wait_until_k8s_ready( - control_node: harness.Instance, instances: List[harness.Instance], - retries: int = 15, delay_s: int = 5, + control_node: harness.Instance, + instances: List[harness.Instance], + retries: int = 15, + delay_s: int = 5, ): """ Validates that the K8s node is in Ready state. From 1bb2e8791d6c273ed61cba5b2184b4671c8442f7 Mon Sep 17 00:00:00 2001 From: "Homayoon (Hue) Alimohammadi" Date: Thu, 25 Jul 2024 13:42:38 +0400 Subject: [PATCH 16/19] Reduce retries on smoke test --- tests/integration/tests/test_smoke.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/tests/test_smoke.py b/tests/integration/tests/test_smoke.py index 57a79e3b6..318705623 100644 --- a/tests/integration/tests/test_smoke.py +++ b/tests/integration/tests/test_smoke.py @@ -24,7 +24,7 @@ def test_smoke(instances: List[harness.Instance]): ) instance.exec(["k8s", "bootstrap", "--file", bootstrap_smoke_config_path]) - util.wait_until_k8s_ready(instance, [instance], retries=120, delay_s=10) + util.wait_until_k8s_ready(instance, [instance]) # Verify the functionality of the k8s config command during the smoke test. # It would be excessive to deploy a cluster solely for this purpose. From a123056c3094c08b1f44da9664a27512f84bb308 Mon Sep 17 00:00:00 2001 From: k8s-bot Date: Fri, 26 Jul 2024 12:19:22 +0400 Subject: [PATCH 17/19] Fix integration tests --- tests/integration/tests/test_smoke.py | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/integration/tests/test_smoke.py b/tests/integration/tests/test_smoke.py index 318705623..1a2baf8a1 100644 --- a/tests/integration/tests/test_smoke.py +++ b/tests/integration/tests/test_smoke.py @@ -4,6 +4,7 @@ import json import logging import re +import time from typing import List import pytest @@ -95,7 +96,7 @@ def test_smoke(instances: List[harness.Instance]): ), "Token not found in the generate-join-token response." # Verify output of the k8s status - result = instance.exec(["k8s", "status"], capture_output=True) + result = instance.exec(["k8s", "status", "--wait-ready"], capture_output=True) patterns = [ r"cluster status:\s*ready", r"control plane nodes:\s*(\d{1,3}(?:\.\d{1,3}){3}:\d{1,5})\s\(voter\)", @@ -108,5 +109,25 @@ def test_smoke(instances: List[harness.Instance]): r"local-storage:\s*enabled at /var/snap/k8s/common/rawfile-storage", r"gateway\s*enabled", ] - for line, pattern in zip(result.stdout.decode().split("\n"), patterns): - assert re.search(pattern, line) + assert len(result.stdout.decode().strip().split("\n")) == len(patterns) + + for i in range(len(patterns)): + timeout = 120 # seconds + t0 = time.time() + while ( + time.time() - t0 < timeout + ): # because some features might take time to get enabled + result_lines = ( + instance.exec(["k8s", "status", "--wait-ready"], capture_output=True) + .stdout.decode().strip() + .split("\n") + ) + line, pattern = result_lines[i], patterns[i] + if re.search(pattern, line) is not None: + break + LOG.info(f"Waiting for \"{line}\" to change...") + time.sleep(10) + else: + assert ( + re.search(pattern, line) is not None + ), f'"Wait timed out. {pattern}" not found in "{line}"' From a1541383e7fd579a390dd9d341aeba7615706f1e Mon Sep 17 00:00:00 2001 From: k8s-bot Date: Fri, 26 Jul 2024 12:22:17 +0400 Subject: [PATCH 18/19] Fix black --- tests/integration/tests/test_smoke.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/tests/test_smoke.py b/tests/integration/tests/test_smoke.py index 1a2baf8a1..e66994048 100644 --- a/tests/integration/tests/test_smoke.py +++ b/tests/integration/tests/test_smoke.py @@ -119,13 +119,14 @@ def test_smoke(instances: List[harness.Instance]): ): # because some features might take time to get enabled result_lines = ( instance.exec(["k8s", "status", "--wait-ready"], capture_output=True) - .stdout.decode().strip() + .stdout.decode() + .strip() .split("\n") ) line, pattern = result_lines[i], patterns[i] if re.search(pattern, line) is not None: break - LOG.info(f"Waiting for \"{line}\" to change...") + LOG.info(f'Waiting for "{line}" to change...') time.sleep(10) else: assert ( From bbc06192c37ce6e07538109ed3c953183fbc179b Mon Sep 17 00:00:00 2001 From: k8s-bot Date: Mon, 29 Jul 2024 10:25:29 +0400 Subject: [PATCH 19/19] Address comments and issues --- .gitignore | 2 - src/k8s/pkg/k8sd/controllers/feature.go | 3 +- src/k8s/pkg/k8sd/features/calico/network.go | 70 +++++++----- src/k8s/pkg/k8sd/features/cilium/gateway.go | 101 +++++++++++------- src/k8s/pkg/k8sd/features/cilium/ingress.go | 63 ++++++----- .../pkg/k8sd/features/cilium/loadbalancer.go | 52 +++++---- src/k8s/pkg/k8sd/features/cilium/network.go | 98 ++++++++++------- src/k8s/pkg/k8sd/features/contour/gateway.go | 58 +++++----- src/k8s/pkg/k8sd/features/contour/ingress.go | 95 +++++++++------- src/k8s/pkg/k8sd/features/coredns/coredns.go | 58 +++++----- src/k8s/pkg/k8sd/features/localpv/localpv.go | 47 ++++---- .../pkg/k8sd/features/metallb/loadbalancer.go | 52 +++++---- .../features/metrics-server/metrics_server.go | 37 ++++--- 13 files changed, 437 insertions(+), 299 deletions(-) diff --git a/.gitignore b/.gitignore index 976bbd3fa..8c7172b67 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,6 @@ /parts/ /stage/ /prime/ -.vscode/ -env/ **.snap diff --git a/src/k8s/pkg/k8sd/controllers/feature.go b/src/k8s/pkg/k8sd/controllers/feature.go index b835411bc..04e50cb6b 100644 --- a/src/k8s/pkg/k8sd/controllers/feature.go +++ b/src/k8s/pkg/k8sd/controllers/feature.go @@ -131,8 +131,7 @@ func (c *FeatureController) reconcile( return fmt.Errorf("failed to retrieve cluster configuration: %w", err) } - featureStatus, err := apply(cfg) - if err != nil { + if featureStatus, err := apply(cfg); err != nil { return fmt.Errorf("failed to apply configuration: %w", err) } else if err := updateFeatureStatus(ctx, featureStatus); err != nil { return fmt.Errorf("failed to update feature status: %w", err) diff --git a/src/k8s/pkg/k8sd/features/calico/network.go b/src/k8s/pkg/k8sd/features/calico/network.go index 0cc8a4509..da1816e61 100644 --- a/src/k8s/pkg/k8sd/features/calico/network.go +++ b/src/k8s/pkg/k8sd/features/calico/network.go @@ -24,39 +24,44 @@ const ( // ApplyNetwork returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annotations types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: calicoTag, - Enabled: cfg.GetEnabled(), - } - m := snap.HelmClient() if !cfg.GetEnabled() { if _, err := m.Apply(ctx, chartCalico, helm.StateDeleted, nil); err != nil { - applyErr := fmt.Errorf("failed to uninstall network: %w", err) - status.Message = fmt.Sprintf(deleteFailedMsgTmpl, applyErr) - return status, applyErr + err = fmt.Errorf("failed to uninstall network: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: calicoTag, + Message: fmt.Sprintf(deleteFailedMsgTmpl, err), + }, err } - status.Version = "" - status.Message = disabledMsg - return status, nil + + return types.FeatureStatus{ + Enabled: false, + Version: calicoTag, + Message: disabledMsg, + }, nil } config, err := internalConfig(annotations) if err != nil { - cfgErr := fmt.Errorf("failed to parse annotations: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, cfgErr) - status.Enabled = false - return status, cfgErr + err = fmt.Errorf("failed to parse annotations: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: calicoTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, err } podIpPools := []map[string]any{} ipv4PodCIDR, ipv6PodCIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR()) if err != nil { - cidrErr := fmt.Errorf("invalid pod cidr: %v", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, cidrErr) - status.Enabled = false - return status, cidrErr + err = fmt.Errorf("invalid pod cidr: %v", err) + return types.FeatureStatus{ + Enabled: false, + Version: calicoTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, err } if ipv4PodCIDR != "" { podIpPools = append(podIpPools, map[string]any{ @@ -76,10 +81,12 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota serviceCIDRs := []string{} ipv4ServiceCIDR, ipv6ServiceCIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR()) if err != nil { - cidrErr := fmt.Errorf("invalid service cidr: %v", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, cidrErr) - status.Enabled = false - return status, cidrErr + err = fmt.Errorf("invalid service cidr: %v", err) + return types.FeatureStatus{ + Enabled: false, + Version: calicoTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, err } if ipv4ServiceCIDR != "" { serviceCIDRs = append(serviceCIDRs, ipv4ServiceCIDR) @@ -121,12 +128,17 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, annota } if _, err := m.Apply(ctx, chartCalico, helm.StatePresent, values); err != nil { - enableErr := fmt.Errorf("failed to enable network: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to enable network: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: calicoTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, err } - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: calicoTag, + Message: enabledMsg, + }, nil } diff --git a/src/k8s/pkg/k8sd/features/cilium/gateway.go b/src/k8s/pkg/k8sd/features/cilium/gateway.go index 6c0a06a1a..892b6be6f 100644 --- a/src/k8s/pkg/k8sd/features/cilium/gateway.go +++ b/src/k8s/pkg/k8sd/features/cilium/gateway.go @@ -23,76 +23,99 @@ const ( // ApplyGateway returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: ciliumAgentImageTag, - Enabled: gateway.GetEnabled(), - } m := snap.HelmClient() if _, err := m.Apply(ctx, chartGateway, helm.StatePresentOrDeleted(gateway.GetEnabled()), nil); err != nil { if gateway.GetEnabled() { - enableErr := fmt.Errorf("failed to install Gateway API CRDs: %w", err) - status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to install Gateway API CRDs: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + }, err } else { - disableErr := fmt.Errorf("failed to delete Gateway API CRDs: %w", err) - status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, disableErr) - return status, disableErr + err = fmt.Errorf("failed to delete Gateway API CRDs: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(gatewayDeleteFailedMsgTmpl, err), + }, err } } // Apply our GatewayClass named ck-gateway if _, err := m.Apply(ctx, chartGatewayClass, helm.StatePresentOrDeleted(gateway.GetEnabled()), nil); err != nil { if gateway.GetEnabled() { - enableErr := fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) - status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + }, err } else { - disableErr := fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) - status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, disableErr) - return status, disableErr + err = fmt.Errorf("failed to install Gateway API GatewayClass: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(gatewayDeleteFailedMsgTmpl, err), + }, err } } changed, err := m.Apply(ctx, chartCilium, helm.StateUpgradeOnlyOrDeleted(network.GetEnabled()), map[string]any{"gatewayAPI": map[string]any{"enabled": gateway.GetEnabled()}}) if err != nil { if gateway.GetEnabled() { - enableErr := fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) - status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + }, err } else { - disableErr := fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) - status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, disableErr) - return status, disableErr + err = fmt.Errorf("failed to apply Gateway API cilium configuration: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(gatewayDeleteFailedMsgTmpl, err), + }, err } } if !changed { if gateway.GetEnabled() { - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: enabledMsg, + }, nil } else { - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: disabledMsg, + }, nil } } if !gateway.GetEnabled() { - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: disabledMsg, + }, nil } if err := rolloutRestartCilium(ctx, snap, 3); err != nil { - resErr := fmt.Errorf("failed to rollout restart cilium to apply Gateway API: %w", err) - status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, resErr) - status.Enabled = false - return status, resErr + err = fmt.Errorf("failed to rollout restart cilium to apply Gateway API: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + }, err } - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: enabledMsg, + }, nil } diff --git a/src/k8s/pkg/k8sd/features/cilium/ingress.go b/src/k8s/pkg/k8sd/features/cilium/ingress.go index 868899324..a05c4b690 100644 --- a/src/k8s/pkg/k8sd/features/cilium/ingress.go +++ b/src/k8s/pkg/k8sd/features/cilium/ingress.go @@ -23,10 +23,6 @@ const ( // ApplyIngress returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: ciliumAgentImageTag, - Enabled: ingress.GetEnabled(), - } m := snap.HelmClient() var values map[string]any @@ -54,41 +50,58 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, ne changed, err := m.Apply(ctx, chartCilium, helm.StateUpgradeOnlyOrDeleted(network.GetEnabled()), values) if err != nil { if network.GetEnabled() { - enableErr := fmt.Errorf("failed to enable ingress: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to enable ingress: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } else { - disableErr := fmt.Errorf("failed to disable ingress: %w", err) - status.Message = fmt.Sprintf(ingressDeleteFailedMsgTmpl, disableErr) - return status, disableErr + err = fmt.Errorf("failed to disable ingress: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(ingressDeleteFailedMsgTmpl, err), + }, err } } if !changed { if ingress.GetEnabled() { - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: enabledMsg, + }, nil } else { - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: disabledMsg, + }, nil } } if !ingress.GetEnabled() { - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: disabledMsg, + }, nil } if err := rolloutRestartCilium(ctx, snap, 3); err != nil { - restartErr := fmt.Errorf("failed to rollout restart cilium to apply ingress: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, restartErr) - status.Enabled = false - return status, restartErr + err = fmt.Errorf("failed to rollout restart cilium to apply ingress: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: enabledMsg, + }, nil } diff --git a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go index b7dc05720..8711ab99a 100644 --- a/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/cilium/loadbalancer.go @@ -25,38 +25,50 @@ const ( // ApplyLoadBalancer returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: ciliumAgentImageTag, - Enabled: loadbalancer.GetEnabled(), - } - if !loadbalancer.GetEnabled() { if err := disableLoadBalancer(ctx, snap, network); err != nil { - disErr := fmt.Errorf("failed to disable LoadBalancer: %w", err) - status.Message = fmt.Sprintf(lbDeleteFailedMsgTmpl, disErr) - return status, disErr + err = fmt.Errorf("failed to disable LoadBalancer: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(lbDeleteFailedMsgTmpl, err), + }, err } - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: disabledMsg, + }, nil } if err := enableLoadBalancer(ctx, snap, loadbalancer, network); err != nil { - enableErr := fmt.Errorf("failed to enable LoadBalancer: %w", err) - status.Message = fmt.Sprintf(lbDeployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to enable LoadBalancer: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(lbDeployFailedMsgTmpl, err), + }, err } if loadbalancer.GetBGPMode() { - status.Message = fmt.Sprintf(lbEnabledMsgTmpl, "BGP") + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(lbEnabledMsgTmpl, "BGP"), + }, nil } else if loadbalancer.GetL2Mode() { - status.Message = fmt.Sprintf(lbEnabledMsgTmpl, "L2") + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(lbEnabledMsgTmpl, "L2"), + }, nil } else { - status.Message = fmt.Sprintf(lbEnabledMsgTmpl, "Unknown") + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(lbEnabledMsgTmpl, "Unknown"), + }, nil } - - return status, nil } func disableLoadBalancer(ctx context.Context, snap snap.Snap, network types.Network) error { diff --git a/src/k8s/pkg/k8sd/features/cilium/network.go b/src/k8s/pkg/k8sd/features/cilium/network.go index 8268d5251..82b010fe8 100644 --- a/src/k8s/pkg/k8sd/features/cilium/network.go +++ b/src/k8s/pkg/k8sd/features/cilium/network.go @@ -26,29 +26,32 @@ const ( // ApplyNetwork returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: ciliumAgentImageTag, - Enabled: cfg.GetEnabled(), - } m := snap.HelmClient() if !cfg.GetEnabled() { if _, err := m.Apply(ctx, chartCilium, helm.StateDeleted, nil); err != nil { - uninstallErr := fmt.Errorf("failed to uninstall network: %w", err) - status.Message = fmt.Sprintf(networkDeleteFailedMsgTmpl, uninstallErr) - return status, uninstallErr + err = fmt.Errorf("failed to uninstall network: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeleteFailedMsgTmpl, err), + }, err } - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: disabledMsg, + }, nil } ipv4CIDR, ipv6CIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR()) if err != nil { - cidrErr := fmt.Errorf("invalid kube-proxy --cluster-cidr value: %v", err) - status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, cidrErr) - status.Enabled = false - return status, cidrErr + err = fmt.Errorf("invalid kube-proxy --cluster-cidr value: %v", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeployFailedMsgTmpl, err), + }, err } values := map[string]any{ @@ -93,18 +96,22 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type if snap.Strict() { bpfMnt, err := utils.GetMountPath("bpf") if err != nil { - mntErr := fmt.Errorf("failed to get bpf mount path: %w", err) - status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, mntErr) - status.Enabled = false - return status, mntErr + err = fmt.Errorf("failed to get bpf mount path: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeployFailedMsgTmpl, err), + }, err } cgrMnt, err := utils.GetMountPath("cgroup2") if err != nil { - cgrpErr := fmt.Errorf("failed to get cgroup2 mount path: %w", err) - status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, cgrpErr) - status.Enabled = false - return status, cgrpErr + err = fmt.Errorf("failed to get cgroup2 mount path: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeployFailedMsgTmpl, err), + }, err } values["bpf"] = map[string]any{ @@ -122,10 +129,12 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type } else { p, err := utils.GetMountPropagation("/sys") if err != nil { - mntErr := fmt.Errorf("failed to get mount propagation for %s: %w", p, err) - status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, mntErr) - status.Enabled = false - return status, mntErr + err = fmt.Errorf("failed to get mount propagation for %s: %w", p, err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeployFailedMsgTmpl, err), + }, err } if p == "private" { onLXD, err := snap.OnLXD(ctx) @@ -133,28 +142,37 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, cfg types.Network, _ type log.FromContext(ctx).Error(err, "Failed to check if running on LXD") } if onLXD { - lxdErr := fmt.Errorf("/sys is not a shared mount on the LXD container, this might be resolved by updating LXD on the host to version 5.0.2 or newer") - status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, lxdErr) - status.Enabled = false - return status, lxdErr + err := fmt.Errorf("/sys is not a shared mount on the LXD container, this might be resolved by updating LXD on the host to version 5.0.2 or newer") + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeployFailedMsgTmpl, err), + }, err } - sysErr := fmt.Errorf("/sys is not a shared mount") - status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, sysErr) - status.Enabled = false - return status, sysErr + err = fmt.Errorf("/sys is not a shared mount") + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeployFailedMsgTmpl, err), + }, err } } if _, err := m.Apply(ctx, chartCilium, helm.StatePresent, values); err != nil { - enableErr := fmt.Errorf("failed to enable network: %w", err) - status.Message = fmt.Sprintf(networkDeployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to enable network: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: ciliumAgentImageTag, + Message: fmt.Sprintf(networkDeployFailedMsgTmpl, err), + }, err } - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: ciliumAgentImageTag, + Message: enabledMsg, + }, nil } func rolloutRestartCilium(ctx context.Context, snap snap.Snap, attempts int) error { diff --git a/src/k8s/pkg/k8sd/features/contour/gateway.go b/src/k8s/pkg/k8sd/features/contour/gateway.go index cf66b8390..d7b203c6b 100644 --- a/src/k8s/pkg/k8sd/features/contour/gateway.go +++ b/src/k8s/pkg/k8sd/features/contour/gateway.go @@ -25,36 +25,41 @@ const ( // ApplyGateway returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: contourGatewayProvisionerContourImageTag, - Enabled: gateway.GetEnabled(), - } m := snap.HelmClient() if !gateway.GetEnabled() { if _, err := m.Apply(ctx, chartGateway, helm.StateDeleted, nil); err != nil { - delErr := fmt.Errorf("failed to uninstall the contour gateway chart: %w", err) - status.Message = fmt.Sprintf(gatewayDeleteFailedMsgTmpl, delErr) - return status, delErr + err = fmt.Errorf("failed to uninstall the contour gateway chart: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourGatewayProvisionerContourImageTag, + Message: fmt.Sprintf(gatewayDeleteFailedMsgTmpl, err), + }, err } - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: contourGatewayProvisionerContourImageTag, + Message: disabledMsg, + }, nil } // Apply common contour CRDS, these are shared with ingress if err := applyCommonContourCRDS(ctx, snap, true); err != nil { - crdErr := fmt.Errorf("failed to apply common contour CRDS: %w", err) - status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, crdErr) - status.Enabled = false - return status, crdErr + err = fmt.Errorf("failed to apply common contour CRDS: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourGatewayProvisionerContourImageTag, + Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + }, err } if err := waitForRequiredContourCommonCRDs(ctx, snap); err != nil { - waitErr := fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) - status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, waitErr) - status.Enabled = false - return status, waitErr + err = fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourGatewayProvisionerContourImageTag, + Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + }, err } values := map[string]any{ @@ -73,14 +78,19 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne } if _, err := m.Apply(ctx, chartGateway, helm.StatePresent, values); err != nil { - installErr := fmt.Errorf("failed to install the contour gateway chart: %w", err) - status.Message = fmt.Sprintf(gatewayDeployFailedMsgTmpl, installErr) - status.Enabled = false - return status, installErr + err = fmt.Errorf("failed to install the contour gateway chart: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourGatewayProvisionerContourImageTag, + Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + }, err } - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: contourGatewayProvisionerContourImageTag, + Message: enabledMsg, + }, nil } // waitForRequiredContourCommonCRDs waits for the required contour CRDs to be available diff --git a/src/k8s/pkg/k8sd/features/contour/ingress.go b/src/k8s/pkg/k8sd/features/contour/ingress.go index b501cebb7..a43023d1c 100644 --- a/src/k8s/pkg/k8sd/features/contour/ingress.go +++ b/src/k8s/pkg/k8sd/features/contour/ingress.go @@ -26,36 +26,41 @@ const ( // returned FeatureStatus. // Contour CRDS are applied through a ck-contour common chart (Overlap with gateway) func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ types.Network, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: contourIngressContourImageTag, - Enabled: ingress.GetEnabled(), - } m := snap.HelmClient() if !ingress.GetEnabled() { if _, err := m.Apply(ctx, chartContour, helm.StateDeleted, nil); err != nil { - delErr := fmt.Errorf("failed to uninstall ingress: %w", err) - status.Message = fmt.Sprintf(ingressDeleteFailedMsgTmpl, delErr) - return status, delErr + err = fmt.Errorf("failed to uninstall ingress: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: fmt.Sprintf(ingressDeleteFailedMsgTmpl, err), + }, err } - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: disabledMsg, + }, nil } // Apply common contour CRDS, these are shared with gateway if err := applyCommonContourCRDS(ctx, snap, true); err != nil { - crdErr := fmt.Errorf("failed to apply common contour CRDS: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, crdErr) - status.Enabled = false - return status, crdErr + err = fmt.Errorf("failed to apply common contour CRDS: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } if err := waitForRequiredContourCommonCRDs(ctx, snap); err != nil { - waitErr := fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, waitErr) - status.Enabled = false - return status, waitErr + err = fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } var values map[string]any @@ -92,18 +97,22 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ changed, err := m.Apply(ctx, chartContour, helm.StatePresent, values) if err != nil { - enableErr := fmt.Errorf("failed to enable ingress: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to enable ingress: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } if changed { if err := rolloutRestartContour(ctx, snap, 3); err != nil { - resErr := fmt.Errorf("failed to rollout restart contour to apply ingress: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, resErr) - status.Enabled = false - return status, resErr + err = fmt.Errorf("failed to rollout restart contour to apply ingress: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } } @@ -115,25 +124,35 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ "defaultTLSSecret": ingress.GetDefaultTLSSecret(), } if _, err := m.Apply(ctx, chartDefaultTLS, helm.StatePresent, values); err != nil { - tlsErr := fmt.Errorf("failed to install the delegation resource for default TLS secret: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, tlsErr) - status.Enabled = false - return status, tlsErr + err = fmt.Errorf("failed to install the delegation resource for default TLS secret: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: contourIngressContourImageTag, + Message: enabledMsg, + }, nil } if _, err := m.Apply(ctx, chartDefaultTLS, helm.StateDeleted, nil); err != nil { - tlsErr := fmt.Errorf("failed to uninstall the delegation resource for default TLS secret: %w", err) - status.Message = fmt.Sprintf(ingressDeployFailedMsgTmpl, tlsErr) - status.Enabled = false - return status, tlsErr + err = fmt.Errorf("failed to uninstall the delegation resource for default TLS secret: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: contourIngressContourImageTag, + Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + }, err } - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: contourIngressContourImageTag, + Message: enabledMsg, + }, nil } // applyCommonContourCRDS will install the common contour CRDS when enabled is true. diff --git a/src/k8s/pkg/k8sd/features/coredns/coredns.go b/src/k8s/pkg/k8sd/features/coredns/coredns.go index 0761a1201..28402e707 100644 --- a/src/k8s/pkg/k8sd/features/coredns/coredns.go +++ b/src/k8s/pkg/k8sd/features/coredns/coredns.go @@ -26,21 +26,22 @@ const ( // ApplyDNS returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types.Kubelet, _ types.Annotations) (types.FeatureStatus, string, error) { - status := types.FeatureStatus{ - Version: imageTag, - Enabled: dns.GetEnabled(), - } m := snap.HelmClient() if !dns.GetEnabled() { if _, err := m.Apply(ctx, chart, helm.StateDeleted, nil); err != nil { - delErr := fmt.Errorf("failed to uninstall coredns: %w", err) - status.Message = fmt.Sprintf(deleteFailedMsgTmpl, delErr) - return status, "", delErr + err = fmt.Errorf("failed to uninstall coredns: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deleteFailedMsgTmpl, err), + }, "", err } - status.Message = disabledMsg - status.Version = "" - return status, "", nil + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: disabledMsg, + }, "", nil } values := map[string]any{ @@ -82,27 +83,36 @@ func ApplyDNS(ctx context.Context, snap snap.Snap, dns types.DNS, kubelet types. } if _, err := m.Apply(ctx, chart, helm.StatePresent, values); err != nil { - applyErr := fmt.Errorf("failed to apply coredns: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, applyErr) - status.Enabled = false - return status, "", applyErr + err = fmt.Errorf("failed to apply coredns: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, "", err } client, err := snap.KubernetesClient("") if err != nil { - clientErr := fmt.Errorf("failed to create kubernetes client: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, clientErr) - status.Enabled = false - return status, "", clientErr + err = fmt.Errorf("failed to create kubernetes client: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, "", err } dnsIP, err := client.GetServiceClusterIP(ctx, "coredns", "kube-system") if err != nil { - retErr := fmt.Errorf("failed to retrieve the coredns service: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, retErr) - status.Enabled = false - return status, "", retErr + err = fmt.Errorf("failed to retrieve the coredns service: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, "", err } - status.Message = fmt.Sprintf(enabledMsgTmpl, dnsIP) - return status, dnsIP, nil + return types.FeatureStatus{ + Enabled: true, + Version: imageTag, + Message: fmt.Sprintf(enabledMsgTmpl, dnsIP), + }, dnsIP, err } diff --git a/src/k8s/pkg/k8sd/features/localpv/localpv.go b/src/k8s/pkg/k8sd/features/localpv/localpv.go index 6439d2c7d..bd812b443 100644 --- a/src/k8s/pkg/k8sd/features/localpv/localpv.go +++ b/src/k8s/pkg/k8sd/features/localpv/localpv.go @@ -23,10 +23,6 @@ const ( // ApplyLocalStorage returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStorage, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: imageTag, - Enabled: cfg.GetEnabled(), - } m := snap.HelmClient() values := map[string]any{ @@ -62,26 +58,35 @@ func ApplyLocalStorage(ctx context.Context, snap snap.Snap, cfg types.LocalStora }, } - _, err := m.Apply(ctx, chart, helm.StatePresentOrDeleted(cfg.GetEnabled()), values) - if err != nil { + if _, err := m.Apply(ctx, chart, helm.StatePresentOrDeleted(cfg.GetEnabled()), values); err != nil { if cfg.GetEnabled() { - enableErr := fmt.Errorf("failed to install rawfile-csi helm package: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to install rawfile-csi helm package: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, err } else { - disableErr := fmt.Errorf("failed to delete rawfile-csi helm package: %w", err) - status.Message = fmt.Sprintf(deleteFailedMsgTmpl, disableErr) - return status, disableErr + err = fmt.Errorf("failed to delete rawfile-csi helm package: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deleteFailedMsgTmpl, err), + }, err } + } + + if cfg.GetEnabled() { + return types.FeatureStatus{ + Enabled: true, + Version: imageTag, + Message: fmt.Sprintf(enabledMsg, cfg.GetLocalPath()), + }, nil } else { - if cfg.GetEnabled() { - status.Message = fmt.Sprintf(enabledMsg, cfg.GetLocalPath()) - return status, nil - } else { - status.Version = "" - status.Message = disabledMsg - return status, nil - } + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: disabledMsg, + }, nil } } diff --git a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go index cb9def6e9..47b583112 100644 --- a/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go +++ b/src/k8s/pkg/k8sd/features/metallb/loadbalancer.go @@ -22,38 +22,50 @@ const ( // ApplyLoadBalancer returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyLoadBalancer(ctx context.Context, snap snap.Snap, loadbalancer types.LoadBalancer, network types.Network, _ types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: controllerImageTag, - Enabled: loadbalancer.GetEnabled(), - } - if !loadbalancer.GetEnabled() { if err := disableLoadBalancer(ctx, snap, network); err != nil { - disableErr := fmt.Errorf("failed to disable LoadBalancer: %w", err) - status.Message = fmt.Sprintf(deleteFailedMsgTmpl, disableErr) - return status, disableErr + err = fmt.Errorf("failed to disable LoadBalancer: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: controllerImageTag, + Message: fmt.Sprintf(deleteFailedMsgTmpl, err), + }, err } - status.Message = disabledMsg - status.Version = "" - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: controllerImageTag, + Message: disabledMsg, + }, nil } if err := enableLoadBalancer(ctx, snap, loadbalancer, network); err != nil { - enableErr := fmt.Errorf("failed to enable LoadBalancer: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to enable LoadBalancer: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: controllerImageTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, err } if loadbalancer.GetBGPMode() { - status.Message = fmt.Sprintf(enabledMsgTmpl, "BGP") + return types.FeatureStatus{ + Enabled: true, + Version: controllerImageTag, + Message: fmt.Sprintf(enabledMsgTmpl, "BGP"), + }, nil } else if loadbalancer.GetL2Mode() { - status.Message = fmt.Sprintf(enabledMsgTmpl, "L2") + return types.FeatureStatus{ + Enabled: true, + Version: controllerImageTag, + Message: fmt.Sprintf(enabledMsgTmpl, "L2"), + }, nil } else { - status.Message = fmt.Sprintf(enabledMsgTmpl, "Unknown") + return types.FeatureStatus{ + Enabled: true, + Version: controllerImageTag, + Message: fmt.Sprintf(enabledMsgTmpl, "Unknown"), + }, nil } - - return status, nil } func disableLoadBalancer(ctx context.Context, snap snap.Snap, network types.Network) error { diff --git a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go index 56fc1c764..48b35ad1e 100644 --- a/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go +++ b/src/k8s/pkg/k8sd/features/metrics-server/metrics_server.go @@ -23,10 +23,6 @@ const ( // ApplyMetricsServer returns an error if anything fails. The error is also wrapped in the .Message field of the // returned FeatureStatus. func ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsServer, annotations types.Annotations) (types.FeatureStatus, error) { - status := types.FeatureStatus{ - Version: imageTag, - Enabled: cfg.GetEnabled(), - } m := snap.HelmClient() config := internalConfig(annotations) @@ -45,22 +41,33 @@ func ApplyMetricsServer(ctx context.Context, snap snap.Snap, cfg types.MetricsSe _, err := m.Apply(ctx, chart, helm.StatePresentOrDeleted(cfg.GetEnabled()), values) if err != nil { if cfg.GetEnabled() { - enableErr := fmt.Errorf("failed to install metrics server chart: %w", err) - status.Message = fmt.Sprintf(deployFailedMsgTmpl, enableErr) - status.Enabled = false - return status, enableErr + err = fmt.Errorf("failed to install metrics server chart: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deployFailedMsgTmpl, err), + }, err } else { - disableErr := fmt.Errorf("failed to delete metrics server chart: %w", err) - status.Message = fmt.Sprintf(deleteFailedMsgTmpl, disableErr) - return status, disableErr + err = fmt.Errorf("failed to delete metrics server chart: %w", err) + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: fmt.Sprintf(deleteFailedMsgTmpl, err), + }, err } } else { if cfg.GetEnabled() { - status.Message = enabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: true, + Version: imageTag, + Message: enabledMsg, + }, nil } else { - status.Message = disabledMsg - return status, nil + return types.FeatureStatus{ + Enabled: false, + Version: imageTag, + Message: disabledMsg, + }, nil } } }