Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Net 6575- Modify Gateways Cleanup Job to cleanup v2 resources #3317

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions charts/consul/templates/gateway-cleanup-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
8 changes: 4 additions & 4 deletions charts/consul/templates/gateway-cleanup-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
159 changes: 150 additions & 9 deletions control-plane/subcommand/gateway-cleanup/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
"errors"
"flag"
"fmt"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extra newline

"io"
"os"
"sync"
"time"

Expand All @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these live in another package? we use them in the resources job so I'm wondering if they belong in the gateways package as constants so we're not defining them twice across this and the resources job

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be in favor of pulling them into a shared package.

)

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)

Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -102,32 +131,59 @@ 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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

// 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
_ = 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 {
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
Expand All @@ -151,8 +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
}

return 0
return nil
}

func (c *Command) validateFlags() error {
Expand Down Expand Up @@ -192,3 +247,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 continue
continue
}
return err
}

// ignore any returned errors
_ = c.k8sClient.Delete(context.Background(), config)

// find the gateway class
gatewayClass := &v2beta1.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) {
// no gateway class, just ignore and continue
continue
}
return err
}

// ignore any returned errors
_ = c.k8sClient.Delete(context.Background(), gatewayClass)

// make sure they're gone
if err := backoff.Retry(func() error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we do this twice? any reason to not just do the entire delete in the retry block to begin with?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since we're just ignoring the errors not having them in the retry block is an attempt to avoid spamming the same delete request while waiting for the finalizers to complete. But I just modified this code to make it into a loop.

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
}
Loading