From df982d5a4b9df83313fd7142813d5e2451120bad Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 6 Dec 2023 14:44:55 -0600 Subject: [PATCH 1/6] cleanup existing gatewayclasses and gatewayclassconfigs --- .../gateway-cleanup-clusterrole.yaml | 9 ++ .../consul/templates/gateway-cleanup-job.yaml | 8 +- .../subcommand/gateway-cleanup/command.go | 133 +++++++++++++++++- 3 files changed, 144 insertions(+), 6 deletions(-) diff --git a/charts/consul/templates/gateway-cleanup-clusterrole.yaml b/charts/consul/templates/gateway-cleanup-clusterrole.yaml index c533a882f5..5518bfc390 100644 --- a/charts/consul/templates/gateway-cleanup-clusterrole.yaml +++ b/charts/consul/templates/gateway-cleanup-clusterrole.yaml @@ -24,6 +24,15 @@ rules: verbs: - get - delete + - apiGroups: + - mesh.consul.hashicorp.com + resources: + - gatewayclassconfigs + - gatewayclasses + - meshgateways + verbs: + - get + - delete {{- if .Values.global.enablePodSecurityPolicies }} - apiGroups: ["policy"] resources: ["podsecuritypolicies"] diff --git a/charts/consul/templates/gateway-cleanup-job.yaml b/charts/consul/templates/gateway-cleanup-job.yaml index 5f8afed3ea..0d4f84272c 100644 --- a/charts/consul/templates/gateway-cleanup-job.yaml +++ b/charts/consul/templates/gateway-cleanup-job.yaml @@ -52,10 +52,10 @@ spec: limits: memory: "50Mi" cpu: "50m" - volumeMounts: - - name: config - mountPath: /consul/config - readOnly: true + volumeMounts: + - name: config + mountPath: /consul/config + readOnly: true {{- if .Values.global.acls.tolerations }} tolerations: {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} diff --git a/control-plane/subcommand/gateway-cleanup/command.go b/control-plane/subcommand/gateway-cleanup/command.go index 583b153d01..647df685f2 100644 --- a/control-plane/subcommand/gateway-cleanup/command.go +++ b/control-plane/subcommand/gateway-cleanup/command.go @@ -8,6 +8,9 @@ import ( "errors" "flag" "fmt" + + "io" + "os" "sync" "time" @@ -19,29 +22,44 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + k8syaml "sigs.k8s.io/yaml" + "github.com/hashicorp/consul-k8s/control-plane/api/mesh/v2beta1" "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" "github.com/hashicorp/consul-k8s/control-plane/subcommand" "github.com/hashicorp/consul-k8s/control-plane/subcommand/flags" ) +const ( + gatewayConfigFilename = "/consul/config/config.yaml" + resourceConfigFilename = "/consul/config/resources.json" +) + type Command struct { UI cli.Ui flags *flag.FlagSet k8s *flags.K8SFlags - flagGatewayClassName string - flagGatewayClassConfigName string + flagGatewayClassName string + flagGatewayClassConfigName string + flagGatewayConfigLocation string + flagResourceConfigFileLocation string k8sClient client.Client once sync.Once help string + gatewayConfig gatewayConfig + ctx context.Context } +type gatewayConfig struct { + GatewayClassConfigs []*v2beta1.GatewayClassConfig `yaml:"gatewayClassConfigs"` +} + func (c *Command) init() { c.flags = flag.NewFlagSet("", flag.ContinueOnError) @@ -50,6 +68,12 @@ func (c *Command) init() { c.flags.StringVar(&c.flagGatewayClassConfigName, "gateway-class-config-name", "", "Name of Kubernetes GatewayClassConfig to delete.") + c.flags.StringVar(&c.flagGatewayConfigLocation, "gateway-config-file-location", gatewayConfigFilename, + "specify a different location for where the gateway config file is") + + c.flags.StringVar(&c.flagResourceConfigFileLocation, "resource-config-file-location", resourceConfigFilename, + "specify a different location for where the gateway resource config file is") + c.k8s = &flags.K8SFlags{} flags.Merge(c.flags, c.k8s.Flags()) c.help = flags.Usage(help, c.flags) @@ -93,6 +117,11 @@ func (c *Command) Run(args []string) int { return 1 } + if err := v2beta1.AddMeshToScheme(s); err != nil { + c.UI.Error(fmt.Sprintf("Could not add consul-k8s schema: %s", err)) + return 1 + } + c.k8sClient, err = client.New(config, client.Options{Scheme: s}) if err != nil { c.UI.Error(fmt.Sprintf("Error initializing Kubernetes client: %s", err)) @@ -102,6 +131,8 @@ func (c *Command) Run(args []string) int { // do the cleanup + //V1 Cleanup + // find the class config and mark it for deletion first so that we // can do an early return if the gateway class isn't found config := &v1alpha1.GatewayClassConfig{} @@ -152,6 +183,18 @@ func (c *Command) Run(args []string) int { // since we don't want to block someone from uninstallation } + //V2 Cleanup + err = c.loadGatewayConfigs() + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + err = c.deleteV2GatewayClassAndClassConfigs() + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + return 0 } @@ -192,3 +235,89 @@ func exponentialBackoffWithMaxIntervalAndTime() *backoff.ExponentialBackOff { backoff.Reset() return backoff } + +// loadGatewayConfigs reads and loads the configs from `/consul/config/config.yaml`, if this file does not exist nothing is done. +func (c *Command) loadGatewayConfigs() error { + file, err := os.Open(c.flagGatewayConfigLocation) + if err != nil { + if os.IsNotExist(err) { + c.UI.Warn(fmt.Sprintf("gateway configuration file not found, skipping gateway configuration, filename: %s", c.flagGatewayConfigLocation)) + return nil + } + c.UI.Error(fmt.Sprintf("Error opening gateway configuration file %s: %s", c.flagGatewayConfigLocation, err)) + return err + } + + config, err := io.ReadAll(file) + if err != nil { + c.UI.Error(fmt.Sprintf("Error reading gateway configuration file %s: %s", c.flagGatewayConfigLocation, err)) + return err + } + + err = k8syaml.Unmarshal(config, &c.gatewayConfig) + if err != nil { + c.UI.Error(fmt.Sprintf("Error decoding gateway config file: %s", err)) + return err + } + + if err := file.Close(); err != nil { + return err + } + return nil +} + +func (c *Command) deleteV2GatewayClassAndClassConfigs() error { + for _, gcc := range c.gatewayConfig.GatewayClassConfigs { + + // find the class config and mark it for deletion first so that we + // can do an early return if the gateway class isn't found + config := &v2beta1.GatewayClassConfig{} + err := c.k8sClient.Get(context.Background(), types.NamespacedName{Name: gcc.Name, Namespace: gcc.Namespace}, config) + if err != nil { + if k8serrors.IsNotFound(err) { + // no gateway class config, just ignore and return + return nil + } + return err + } + + // ignore any returned errors + _ = c.k8sClient.Delete(context.Background(), config) + + // find the gateway class + gatewayClass := &v2beta1.GatewayClass{} + //TODO is the correct name for the gatewayclass? + err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: gcc.Name, Namespace: gcc.Namespace}, gatewayClass) + if err != nil { + if k8serrors.IsNotFound(err) { + // no gateway class, just ignore and return + return nil + } + return err + } + + // ignore any returned errors + _ = c.k8sClient.Delete(context.Background(), gatewayClass) + + // make sure they're gone + if err := backoff.Retry(func() error { + err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: gcc.Name, Namespace: gcc.Namespace}, config) + if err == nil || !k8serrors.IsNotFound(err) { + return errors.New("gateway class config still exists") + } + + err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: gcc.Name, Namespace: gcc.Namespace}, gatewayClass) + if err == nil || !k8serrors.IsNotFound(err) { + return errors.New("gateway class still exists") + } + + return nil + }, exponentialBackoffWithMaxIntervalAndTime()); err != nil { + c.UI.Error(err.Error()) + // if we failed, return 0 anyway after logging the error + // since we don't want to block someone from uninstallation + } + } + + return nil +} From 587fcd323bb8b6a9e4cc9977a3756db45817d2b6 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 6 Dec 2023 15:22:10 -0600 Subject: [PATCH 2/6] add tests for v2 --- .../subcommand/gateway-cleanup/command.go | 4 + .../gateway-cleanup/command_test.go | 115 ++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/control-plane/subcommand/gateway-cleanup/command.go b/control-plane/subcommand/gateway-cleanup/command.go index 647df685f2..f4f474b944 100644 --- a/control-plane/subcommand/gateway-cleanup/command.go +++ b/control-plane/subcommand/gateway-cleanup/command.go @@ -138,6 +138,7 @@ func (c *Command) Run(args []string) int { config := &v1alpha1.GatewayClassConfig{} err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: c.flagGatewayClassConfigName}, config) if err != nil { + if k8serrors.IsNotFound(err) { // no gateway class config, just ignore and return return 0 @@ -150,6 +151,7 @@ func (c *Command) Run(args []string) int { _ = c.k8sClient.Delete(context.Background(), config) // find the gateway class + gatewayClass := &gwv1beta1.GatewayClass{} err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: c.flagGatewayClassName}, gatewayClass) if err != nil { @@ -186,12 +188,14 @@ func (c *Command) Run(args []string) int { //V2 Cleanup err = c.loadGatewayConfigs() if err != nil { + c.UI.Error(err.Error()) return 1 } err = c.deleteV2GatewayClassAndClassConfigs() if err != nil { c.UI.Error(err.Error()) + return 1 } diff --git a/control-plane/subcommand/gateway-cleanup/command_test.go b/control-plane/subcommand/gateway-cleanup/command_test.go index 038c5b5667..fe178ff5f8 100644 --- a/control-plane/subcommand/gateway-cleanup/command_test.go +++ b/control-plane/subcommand/gateway-cleanup/command_test.go @@ -4,6 +4,9 @@ package gatewaycleanup import ( + "github.com/hashicorp/consul-k8s/control-plane/api/mesh/v2beta1" + corev1 "k8s.io/api/core/v1" + "os" "testing" "github.com/mitchellh/cli" @@ -52,6 +55,7 @@ func TestRun(t *testing.T) { s := runtime.NewScheme() require.NoError(t, gwv1beta1.Install(s)) require.NoError(t, v1alpha1.AddToScheme(s)) + require.NoError(t, v2beta1.AddMeshToScheme(s)) objs := []client.Object{} if tt.gatewayClass != nil { @@ -82,3 +86,114 @@ func TestRun(t *testing.T) { }) } } + +func TestRunV2Resources(t *testing.T) { + t.Parallel() + + for name, tt := range map[string]struct { + gatewayClassConfig []*v2beta1.GatewayClassConfig + gatewayClass []*v2beta1.GatewayClass + configMapData string + }{ + + "v2 resources exists": { + gatewayClassConfig: []*v2beta1.GatewayClassConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + }, + }, + }, + gatewayClass: []*v2beta1.GatewayClass{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + }, + }, + }, + configMapData: `gatewayClassConfigs: +- apiVersion: mesh.consul.hashicorp.com/v2beta1 + kind: GatewayClassConfig + metadata: + name: test-gateway + spec: + deployment: + container: + resources: + requests: + cpu: 200m + memory: 200Mi + limits: + cpu: 200m + memory: 200Mi +`, + }, + "v2 emptyconfigmap": { + configMapData: "", + }, + } { + t.Run(name, func(t *testing.T) { + tt := tt + + t.Parallel() + + s := runtime.NewScheme() + require.NoError(t, gwv1beta1.Install(s)) + require.NoError(t, v2beta1.AddMeshToScheme(s)) + require.NoError(t, corev1.AddToScheme(s)) + require.NoError(t, v1alpha1.AddToScheme(s)) + + objs := []client.Object{} + for _, gatewayClass := range tt.gatewayClass { + objs = append(objs, gatewayClass) + } + for _, gatewayClassConfig := range tt.gatewayClassConfig { + objs = append(objs, gatewayClassConfig) + } + + configMap := &corev1.ConfigMap{ + Data: map[string]string{ + "config.yaml": tt.configMapData, + }, + } + + objs = append(objs, configMap) + + client := fake.NewClientBuilder().WithScheme(s).WithObjects(objs...).Build() + + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + k8sClient: client, + flagGatewayClassName: "gateway-class", + flagGatewayClassConfigName: "gateway-class-config", + } + + code := cmd.Run([]string{ + "-gateway-class-config-name", "gateway-class-config", + "-gateway-class-name", "gateway-class", + }) + + require.Equal(t, 0, code) + }) + } +} + +func createGatewayConfigFile(t *testing.T, fileContent, filename string) string { + t.Helper() + + // create a temp file to store configuration yaml + tmpdir := t.TempDir() + file, err := os.CreateTemp(tmpdir, filename) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + _, err = file.WriteString(fileContent) + if err != nil { + t.Fatal(err) + } + + return file.Name() +} From d03de676d31b90dfb678028e37890f0cbddea090 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Thu, 7 Dec 2023 13:23:33 -0600 Subject: [PATCH 3/6] fixed test to actually run against v2 resources, found issue where loop was failing out early --- .../subcommand/gateway-cleanup/command.go | 59 +++++++++------- .../gateway-cleanup/command_test.go | 67 ++++++++++++++++--- 2 files changed, 93 insertions(+), 33 deletions(-) diff --git a/control-plane/subcommand/gateway-cleanup/command.go b/control-plane/subcommand/gateway-cleanup/command.go index f4f474b944..a727b0e7ca 100644 --- a/control-plane/subcommand/gateway-cleanup/command.go +++ b/control-plane/subcommand/gateway-cleanup/command.go @@ -132,19 +132,42 @@ func (c *Command) Run(args []string) int { // do the cleanup //V1 Cleanup + err = c.deleteV1GatewayClassAndGatewayClasConfig() + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + + //V2 Cleanup + err = c.loadGatewayConfigs() + if err != nil { + + c.UI.Error(err.Error()) + return 1 + } + err = c.deleteV2GatewayClassAndClassConfigs() + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + + return 0 +} + +func (c *Command) deleteV1GatewayClassAndGatewayClasConfig() error { // find the class config and mark it for deletion first so that we // can do an early return if the gateway class isn't found config := &v1alpha1.GatewayClassConfig{} - err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: c.flagGatewayClassConfigName}, config) + err := c.k8sClient.Get(context.Background(), types.NamespacedName{Name: c.flagGatewayClassConfigName}, config) if err != nil { if k8serrors.IsNotFound(err) { // no gateway class config, just ignore and return - return 0 + return nil } c.UI.Error(err.Error()) - return 1 + return err } // ignore any returned errors @@ -157,10 +180,10 @@ func (c *Command) Run(args []string) int { if err != nil { if k8serrors.IsNotFound(err) { // no gateway class, just ignore and return - return 0 + return nil } c.UI.Error(err.Error()) - return 1 + return err } // ignore any returned errors @@ -184,22 +207,7 @@ func (c *Command) Run(args []string) int { // if we failed, return 0 anyway after logging the error // since we don't want to block someone from uninstallation } - - //V2 Cleanup - err = c.loadGatewayConfigs() - if err != nil { - - c.UI.Error(err.Error()) - return 1 - } - err = c.deleteV2GatewayClassAndClassConfigs() - if err != nil { - c.UI.Error(err.Error()) - - return 1 - } - - return 0 + return nil } func (c *Command) validateFlags() error { @@ -245,6 +253,7 @@ func (c *Command) loadGatewayConfigs() error { file, err := os.Open(c.flagGatewayConfigLocation) if err != nil { if os.IsNotExist(err) { + panic(err) c.UI.Warn(fmt.Sprintf("gateway configuration file not found, skipping gateway configuration, filename: %s", c.flagGatewayConfigLocation)) return nil } @@ -279,8 +288,8 @@ func (c *Command) deleteV2GatewayClassAndClassConfigs() error { err := c.k8sClient.Get(context.Background(), types.NamespacedName{Name: gcc.Name, Namespace: gcc.Namespace}, config) if err != nil { if k8serrors.IsNotFound(err) { - // no gateway class config, just ignore and return - return nil + // no gateway class config, just ignore and continue + continue } return err } @@ -294,8 +303,8 @@ func (c *Command) deleteV2GatewayClassAndClassConfigs() error { err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: gcc.Name, Namespace: gcc.Namespace}, gatewayClass) if err != nil { if k8serrors.IsNotFound(err) { - // no gateway class, just ignore and return - return nil + // no gateway class, just ignore and continue + continue } return err } diff --git a/control-plane/subcommand/gateway-cleanup/command_test.go b/control-plane/subcommand/gateway-cleanup/command_test.go index fe178ff5f8..69c626db6c 100644 --- a/control-plane/subcommand/gateway-cleanup/command_test.go +++ b/control-plane/subcommand/gateway-cleanup/command_test.go @@ -126,6 +126,62 @@ func TestRunV2Resources(t *testing.T) { limits: cpu: 200m memory: 200Mi +`, + }, + "multiple v2 resources exists": { + gatewayClassConfig: []*v2beta1.GatewayClassConfig{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway2", + }, + }, + }, + gatewayClass: []*v2beta1.GatewayClass{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway2", + }, + }, + }, + configMapData: `gatewayClassConfigs: +- apiVersion: mesh.consul.hashicorp.com/v2beta1 + kind: GatewayClassConfig + metadata: + name: test-gateway + spec: + deployment: + container: + resources: + requests: + cpu: 200m + memory: 200Mi + limits: + cpu: 200m + memory: 200Mi +- apiVersion: mesh.consul.hashicorp.com/v2beta1 + kind: GatewayClassConfig + metadata: + name: test-gateway2 + spec: + deployment: + container: + resources: + requests: + cpu: 200m + memory: 200Mi + limits: + cpu: 200m + memory: 200Mi `, }, "v2 emptyconfigmap": { @@ -151,13 +207,7 @@ func TestRunV2Resources(t *testing.T) { objs = append(objs, gatewayClassConfig) } - configMap := &corev1.ConfigMap{ - Data: map[string]string{ - "config.yaml": tt.configMapData, - }, - } - - objs = append(objs, configMap) + path := createGatewayConfigFile(t, tt.configMapData, "config.yaml") client := fake.NewClientBuilder().WithScheme(s).WithObjects(objs...).Build() @@ -167,11 +217,13 @@ func TestRunV2Resources(t *testing.T) { k8sClient: client, flagGatewayClassName: "gateway-class", flagGatewayClassConfigName: "gateway-class-config", + flagGatewayConfigLocation: path, } code := cmd.Run([]string{ "-gateway-class-config-name", "gateway-class-config", "-gateway-class-name", "gateway-class", + "-gateway-config-file-location", path, }) require.Equal(t, 0, code) @@ -194,6 +246,5 @@ func createGatewayConfigFile(t *testing.T, fileContent, filename string) string if err != nil { t.Fatal(err) } - return file.Name() } From 837c1daf3c7ce5f540bc4024d08413f4775022ea Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 8 Dec 2023 10:43:53 -0600 Subject: [PATCH 4/6] add TODO with jira ticket --- control-plane/subcommand/gateway-cleanup/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/subcommand/gateway-cleanup/command.go b/control-plane/subcommand/gateway-cleanup/command.go index a727b0e7ca..2ec57b20ff 100644 --- a/control-plane/subcommand/gateway-cleanup/command.go +++ b/control-plane/subcommand/gateway-cleanup/command.go @@ -299,7 +299,7 @@ func (c *Command) deleteV2GatewayClassAndClassConfigs() error { // find the gateway class gatewayClass := &v2beta1.GatewayClass{} - //TODO is the correct name for the gatewayclass? + //TODO: NET-6838 To pull the GatewayClassName from the Configmap err = c.k8sClient.Get(context.Background(), types.NamespacedName{Name: gcc.Name, Namespace: gcc.Namespace}, gatewayClass) if err != nil { if k8serrors.IsNotFound(err) { From 13a00dd5a2b50ee35b52e11a3bd70890adcf5a42 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 8 Dec 2023 11:21:37 -0600 Subject: [PATCH 5/6] cleanup debug line --- control-plane/subcommand/gateway-cleanup/command.go | 1 - 1 file changed, 1 deletion(-) diff --git a/control-plane/subcommand/gateway-cleanup/command.go b/control-plane/subcommand/gateway-cleanup/command.go index 2ec57b20ff..7a63adc710 100644 --- a/control-plane/subcommand/gateway-cleanup/command.go +++ b/control-plane/subcommand/gateway-cleanup/command.go @@ -253,7 +253,6 @@ func (c *Command) loadGatewayConfigs() error { file, err := os.Open(c.flagGatewayConfigLocation) if err != nil { if os.IsNotExist(err) { - panic(err) c.UI.Warn(fmt.Sprintf("gateway configuration file not found, skipping gateway configuration, filename: %s", c.flagGatewayConfigLocation)) return nil } From c4272b2bf62e7055c16db2bbb0668d9ba24aed23 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Fri, 8 Dec 2023 17:10:03 -0600 Subject: [PATCH 6/6] delete extra newline --- control-plane/subcommand/gateway-cleanup/command.go | 1 - 1 file changed, 1 deletion(-) diff --git a/control-plane/subcommand/gateway-cleanup/command.go b/control-plane/subcommand/gateway-cleanup/command.go index 7a63adc710..5e01ca0d3c 100644 --- a/control-plane/subcommand/gateway-cleanup/command.go +++ b/control-plane/subcommand/gateway-cleanup/command.go @@ -8,7 +8,6 @@ import ( "errors" "flag" "fmt" - "io" "os" "sync"