From 647fb6cb11a706df62d1103f9d55886c7af67a78 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Wed, 6 Mar 2024 09:44:54 -0500 Subject: [PATCH] support byovpc and clean up list filtering --- .github/workflows/build_test_ci.yml | 5 +++ api/v1alpha1/linodecluster_types.go | 2 +- api/v1alpha1/zz_generated.deepcopy.go | 5 +++ cloud/services/loadbalancers.go | 21 +++++----- cloud/services/object_storage_buckets.go | 8 +++- controller/linodecluster_controller.go | 8 ++-- controller/linodemachine_controller.go | 7 +++- controller/linodevpc_controller.go | 10 ++--- controller/linodevpc_controller_helpers.go | 9 ++++- util/filter.go | 45 ++++++++++++++++++++++ util/helpers.go | 24 ------------ 11 files changed, 93 insertions(+), 51 deletions(-) create mode 100644 util/filter.go diff --git a/.github/workflows/build_test_ci.yml b/.github/workflows/build_test_ci.yml index 56c71f98b..7f201aa51 100644 --- a/.github/workflows/build_test_ci.yml +++ b/.github/workflows/build_test_ci.yml @@ -46,6 +46,7 @@ jobs: uses: actions/setup-go@v5 with: go-version: '1.22' + check-latest: true - name: Build run: make build @@ -80,6 +81,7 @@ jobs: uses: actions/setup-go@v5 with: go-version: '1.22' + check-latest: true - name: Docker cache uses: ScribeMD/docker-cache@0.3.7 @@ -135,6 +137,7 @@ jobs: uses: actions/setup-go@v5 with: go-version: '1.22' + check-latest: true - name: Docker cache uses: ScribeMD/docker-cache@0.3.7 @@ -156,6 +159,7 @@ jobs: with: name: logs path: .logs/* + overwrite: true chainsaw-test: needs: [go-build-test, docker-build] @@ -194,6 +198,7 @@ jobs: uses: actions/setup-go@v5 with: go-version: '1.22' + check-latest: true - name: Docker cache uses: ScribeMD/docker-cache@0.3.7 diff --git a/api/v1alpha1/linodecluster_types.go b/api/v1alpha1/linodecluster_types.go index a5424cbec..39840c588 100644 --- a/api/v1alpha1/linodecluster_types.go +++ b/api/v1alpha1/linodecluster_types.go @@ -107,7 +107,7 @@ type NetworkSpec struct { LoadBalancerPort int `json:"loadBalancerPort,omitempty"` // NodeBalancerID is the id of api server NodeBalancer. // +optional - NodeBalancerID int `json:"nodeBalancerID,omitempty"` + NodeBalancerID *int `json:"nodeBalancerID,omitempty"` // NodeBalancerConfigID is the config ID of api server NodeBalancer. // +optional NodeBalancerConfigID *int `json:"nodeBalancerConfigID,omitempty"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7286e06a8..8b8680438 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -790,6 +790,11 @@ func (in *LinodeVPCStatus) DeepCopy() *LinodeVPCStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { *out = *in + if in.NodeBalancerID != nil { + in, out := &in.NodeBalancerID, &out.NodeBalancerID + *out = new(int) + **out = **in + } if in.NodeBalancerConfigID != nil { in, out := &in.NodeBalancerConfigID, &out.NodeBalancerConfigID *out = new(int) diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go index 4a5cdb012..7c6b4bedd 100644 --- a/cloud/services/loadbalancers.go +++ b/cloud/services/loadbalancers.go @@ -2,7 +2,6 @@ package services import ( "context" - "encoding/json" "errors" "fmt" "net/http" @@ -21,20 +20,18 @@ const ( // CreateNodeBalancer creates a new NodeBalancer if one doesn't exist func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { - var linodeNBs []linodego.NodeBalancer var linodeNB *linodego.NodeBalancer + NBLabel := fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name) clusterUID := string(clusterScope.LinodeCluster.UID) tags := []string{string(clusterScope.LinodeCluster.UID)} - filter := map[string]string{ - "label": NBLabel, + listFilter := util.Filter{ + ID: clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + Label: NBLabel, + Tags: tags, } - - rawFilter, err := json.Marshal(filter) + linodeNBs, err := clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, listFilter.String())) if err != nil { - return nil, err - } - if linodeNBs, err = clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { logger.Info("Failed to list NodeBalancers", "error", err.Error()) return nil, err @@ -97,7 +94,7 @@ func CreateNodeBalancerConfig( if linodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig( ctx, - clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, createConfig, ); err != nil { logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) @@ -145,7 +142,7 @@ func AddNodeToNB( } _, err = machineScope.LinodeClient.CreateNodeBalancerNode( ctx, - machineScope.LinodeCluster.Spec.Network.NodeBalancerID, + *machineScope.LinodeCluster.Spec.Network.NodeBalancerID, *machineScope.LinodeCluster.Spec.Network.NodeBalancerConfigID, linodego.NodeBalancerNodeCreateOptions{ Label: machineScope.Cluster.Name, @@ -185,7 +182,7 @@ func DeleteNodeFromNB( err := machineScope.LinodeClient.DeleteNodeBalancerNode( ctx, - machineScope.LinodeCluster.Spec.Network.NodeBalancerID, + *machineScope.LinodeCluster.Spec.Network.NodeBalancerID, *machineScope.LinodeCluster.Spec.Network.NodeBalancerConfigID, *machineScope.LinodeMachine.Spec.InstanceID, ) diff --git a/cloud/services/object_storage_buckets.go b/cloud/services/object_storage_buckets.go index ca94a5fc7..ac82f9deb 100644 --- a/cloud/services/object_storage_buckets.go +++ b/cloud/services/object_storage_buckets.go @@ -16,9 +16,15 @@ import ( ) func EnsureObjectStorageBucket(ctx context.Context, bScope *scope.ObjectStorageBucketScope) (*linodego.ObjectStorageBucket, error) { + // Buckets do not have IDs so hardcode it to 0 + listFilter := util.Filter{ + ID: nil, + Label: *bScope.Bucket.Spec.Label, + Tags: nil, + } buckets, err := bScope.LinodeClient.ListObjectStorageBucketsInCluster( ctx, - linodego.NewListOptions(1, util.CreateLinodeAPIFilter(*bScope.Bucket.Spec.Label, nil)), + linodego.NewListOptions(1, listFilter.String()), bScope.Bucket.Spec.Cluster, ) if err != nil { diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 998917755..aa9e7b748 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -170,7 +170,7 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger lo return err } - clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = linodeNB.ID + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = &linodeNB.ID linodeNBConfig, err := services.CreateNodeBalancerConfig(ctx, clusterScope, logger) if err != nil { @@ -191,14 +191,14 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger lo func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { logger.Info("deleting cluster") - if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == 0 { + if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil { logger.Info("NodeBalancer ID is missing, nothing to do") controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) return nil } - if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { + if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { logger.Info("Failed to delete Linode NodeBalancer", "error", err.Error()) // Not found is not an error @@ -212,7 +212,7 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") - clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = nil clusterScope.LinodeCluster.Spec.Network.NodeBalancerConfigID = nil controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index cdbb97b7f..198c62716 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -307,7 +307,12 @@ func (r *LinodeMachineReconciler) reconcileCreate( tags := []string{machineScope.LinodeCluster.Name} - linodeInstances, err := machineScope.LinodeClient.ListInstances(ctx, linodego.NewListOptions(1, util.CreateLinodeAPIFilter(machineScope.LinodeMachine.Name, tags))) + listFilter := util.Filter{ + ID: machineScope.LinodeMachine.Spec.InstanceID, + Label: machineScope.LinodeMachine.Name, + Tags: tags, + } + linodeInstances, err := machineScope.LinodeClient.ListInstances(ctx, linodego.NewListOptions(1, listFilter.String())) if err != nil { logger.Error(err, "Failed to list Linode machine instances") diff --git a/controller/linodevpc_controller.go b/controller/linodevpc_controller.go index 771c95ae5..df77123c1 100644 --- a/controller/linodevpc_controller.go +++ b/controller/linodevpc_controller.go @@ -56,11 +56,11 @@ type LinodeVPCReconciler struct { ReconcileTimeout time.Duration } -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodevpcs,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodevpcs/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodevpcs/finalizers,verbs=update +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodevpcs,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodevpcs/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodevpcs/finalizers,verbs=update -//+kubebuilder:rbac:groups="",resources=events,verbs=create;update;patch +// +kubebuilder:rbac:groups="",resources=events,verbs=create;update;patch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the VPC closer to the desired state. @@ -221,8 +221,6 @@ func (r *LinodeVPCReconciler) reconcileDelete(ctx context.Context, logger logr.L logger.Error(err, "Failed to fetch VPC") return res, err - } else if vpc == nil { - return res, errors.New("failed to fetch VPC") } if vpc != nil { diff --git a/controller/linodevpc_controller_helpers.go b/controller/linodevpc_controller_helpers.go index 2c168bb85..feed349e5 100644 --- a/controller/linodevpc_controller_helpers.go +++ b/controller/linodevpc_controller_helpers.go @@ -39,18 +39,23 @@ func (r *LinodeVPCReconciler) reconcileVPC(ctx context.Context, vpcScope *scope. return err } - if createConfig.Label == "" { createConfig.Label = vpcScope.LinodeVPC.Name } - if vpcs, err := vpcScope.LinodeClient.ListVPCs(ctx, linodego.NewListOptions(1, util.CreateLinodeAPIFilter(createConfig.Label, nil))); err != nil { + listFilter := util.Filter{ + ID: vpcScope.LinodeVPC.Spec.VPCID, + Label: createConfig.Label, + Tags: nil, + } + if vpcs, err := vpcScope.LinodeClient.ListVPCs(ctx, linodego.NewListOptions(1, listFilter.String())); err != nil { logger.Error(err, "Failed to list VPCs") return err } else if len(vpcs) != 0 { // Labels are unique vpcScope.LinodeVPC.Spec.VPCID = &vpcs[0].ID + vpcScope.LinodeVPC.Spec.Label = vpcs[0].Label return nil } diff --git a/util/filter.go b/util/filter.go new file mode 100644 index 000000000..509f6dfe9 --- /dev/null +++ b/util/filter.go @@ -0,0 +1,45 @@ +package util + +import ( + "encoding/json" + "strconv" + "strings" +) + +// Filter holds the fields used for filtering results from the Linode API. +// +// The fields within Filter are prioritized so that only the most-specific +// field is present when Filter is marshaled to JSON. +type Filter struct { + ID *int // Filter on the resource's ID (most specific). + Label string // Filter on the resource's label. + Tags []string // Filter resources by their tags (least specific). +} + +// MarshalJSON returns a JSON-encoded representation of a [Filter]. +// The resulting encoded value will have exactly 1 (one) field present. +// See [Filter] for details on the value precedence. +func (f Filter) MarshalJSON() ([]byte, error) { + filter := make(map[string]string, 1) + switch { + case f.ID != nil: + filter["id"] = strconv.Itoa(*f.ID) + case f.Label != "": + filter["label"] = f.Label + case len(f.Tags) != 0: + filter["tags"] = strings.Join(f.Tags, ",") + } + + return json.Marshal(filter) +} + +// String returns the string representation of the encoded value from +// [Filter.MarshalJSON]. +func (f Filter) String() string { + p, err := f.MarshalJSON() + if err != nil { + panic("this should not have failed") + } + + return string(p) +} diff --git a/util/helpers.go b/util/helpers.go index 86f472dd2..7836f7ed0 100644 --- a/util/helpers.go +++ b/util/helpers.go @@ -1,9 +1,6 @@ package util import ( - "encoding/json" - "strings" - "github.com/linode/linodego" ) @@ -12,27 +9,6 @@ func Pointer[T any](t T) *T { return &t } -// CreateLinodeAPIFilter converts variables to API filter string -func CreateLinodeAPIFilter(label string, tags []string) string { - filter := map[string]string{} - - if label != "" { - filter["label"] = label - } - - if len(tags) != 0 { - filter["tags"] = strings.Join(tags, ",") - } - - rawFilter, err := json.Marshal(filter) - if err != nil { - // This should never happen - panic(err.Error() + " Oh, snap... Earth has over, we can't parse map[string]string to JSON! I'm going to die ...") - } - - return string(rawFilter) -} - // IgnoreLinodeAPIError returns the error except matches to status code func IgnoreLinodeAPIError(err error, code int) error { apiErr := linodego.Error{Code: code}