diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eb388c..ceb46e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [v1.1.1] - 2024-02-26 +### Fixed +- fix(taints): fixed issue where Node taint was being applied irrespective of whether it was already applied + ## [v1.1.0] - 2024-02-22 ### Added - feat(taint): added custom node taint for matching Nodes that don't have a PodCIDR assigned diff --git a/api/v1alpha1/nodecidrallocation_types.go b/api/v1alpha1/nodecidrallocation_types.go index 3fdf74c..ad924ee 100644 --- a/api/v1alpha1/nodecidrallocation_types.go +++ b/api/v1alpha1/nodecidrallocation_types.go @@ -77,6 +77,7 @@ type NodeCIDRAllocationStatus struct { // This is a CRD that defines a NodeCIDRAllocation resource which allows for the allocation of PodCIDRs / Pod Subnets to Kubernetes Nodes // to be assigned to nodes in a cluster. This is implemented using a list of network CIDRs as blocks of available address space that can be allocated // to nodes using a node selector to filter the nodes upon which to apply the Pod CIDRs. +// // +kubebuilder:printcolumn:name="Created",type="date",JSONPath=".metadata.creationTimestamp",description="NodeCIDRAllocation creation timestamp" // +kubebuilder:printcolumn:name="Pools",type="string",JSONPath=".spec.addressPools",description="NodeCIDRAllocation Address Pools" // +kubebuilder:printcolumn:name="NodeSelector",type="string",JSONPath=".spec.nodeSelector",description="NodeCIDRAllocation NodeSelector value" diff --git a/install/kubernetes/cidr-allocator/Chart.yaml b/install/kubernetes/cidr-allocator/Chart.yaml index cb23984..b575be9 100644 --- a/install/kubernetes/cidr-allocator/Chart.yaml +++ b/install/kubernetes/cidr-allocator/Chart.yaml @@ -10,9 +10,9 @@ keywords: - Networking - Node -version: 2.0.3 +version: 2.0.4 kubeVersion: ">= 1.16.0-0" -appVersion: "v1.1.0" +appVersion: "v1.1.1" maintainers: - name: Ben Sykes diff --git a/install/kubernetes/cidr-allocator/README.md b/install/kubernetes/cidr-allocator/README.md index 64b87a2..06a2349 100644 --- a/install/kubernetes/cidr-allocator/README.md +++ b/install/kubernetes/cidr-allocator/README.md @@ -2,7 +2,7 @@ A Helm chart to deploy the STATCAN CIDR-Allocator Controller and CRDs to a Kubernetes Cluster -![Version: 2.0.3](https://img.shields.io/badge/Version-2.0.3-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.1.0](https://img.shields.io/badge/AppVersion-v1.1.0-informational?style=flat-square) +![Version: 2.0.4](https://img.shields.io/badge/Version-2.0.4-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.1.1](https://img.shields.io/badge/AppVersion-v1.1.1-informational?style=flat-square) A Helm chart to deploy the STATCAN CIDR-Allocator Controller and CRDs to a Kubernetes Cluster diff --git a/install/kubernetes/cidr-allocator/values.yaml b/install/kubernetes/cidr-allocator/values.yaml index 577f228..d2deab3 100644 --- a/install/kubernetes/cidr-allocator/values.yaml +++ b/install/kubernetes/cidr-allocator/values.yaml @@ -11,7 +11,7 @@ image: # -- can be one of "Always", "IfNotPresent", "Never" pullPolicy: IfNotPresent # -- Overrides the image tag whose default is the chart appVersion. - # tag: "v1.0.1" + # tag: "v1.1.1" # -- specifies credentials for a private registry to pull source image imagePullSecrets: [] diff --git a/internal/controller/nodecidrallocation_controller.go b/internal/controller/nodecidrallocation_controller.go index c408fcc..396c2f0 100644 --- a/internal/controller/nodecidrallocation_controller.go +++ b/internal/controller/nodecidrallocation_controller.go @@ -42,12 +42,21 @@ import ( "statcan.gc.ca/cidr-allocator/internal/helper" statcan_metrics "statcan.gc.ca/cidr-allocator/internal/metrics" statcan_net "statcan.gc.ca/cidr-allocator/internal/networking" + "statcan.gc.ca/cidr-allocator/internal/taint" ) const ( finalizerName = "nodecidrallocation.networking.statcan.gc.ca/finalizer" ) +var ( + nodeTaint = corev1.Taint{ + Key: "node.networking.statcan.gc.ca/network-unavailable", + Value: "true", + Effect: corev1.TaintEffectNoSchedule, + } +) + // NodeCIDRAllocationReconciler reconciles a NodeCIDRAllocation object type NodeCIDRAllocationReconciler struct { client.Client @@ -184,7 +193,9 @@ func (r *NodeCIDRAllocationReconciler) Reconcile(ctx context.Context, req ctrl.R if len(matchingNodes.Items) == 0 { rl.V(1).Info("no matching nodes exist. skipping") - return ctrl.Result{}, nil + + // nodeCIDRAllocation does not have any matching nodes - return and do not requeue + return ctrl.Result{}, r.finalizeReconcile(ctx, &nodeCIDRAllocation, &matchingNodes, nil) } // retrieve a list of all Nodes in the cluster. @@ -197,7 +208,7 @@ func (r *NodeCIDRAllocationReconciler) Reconcile(ctx context.Context, req ctrl.R ) // could not list Nodes in the cluster - return and requeue - return ctrl.Result{}, err + return ctrl.Result{}, r.finalizeReconcile(ctx, &nodeCIDRAllocation, &matchingNodes, err) } // @@ -265,7 +276,7 @@ func (r *NodeCIDRAllocationReconciler) Reconcile(ctx context.Context, req ctrl.R &nodeCIDRAllocation, corev1.EventTypeWarning, EventReasonNoAddressSpace, - "There are no available subnets for the requested size (/%s). Could not assign PodCIDR to Node (%s)", requiredCIDRMask, node.GetName(), + "There are no available subnets for the requested size (/%d). Could not assign PodCIDR to Node (%s)", requiredCIDRMask, node.GetName(), ) // no available subnet to assign to Node - return and do not requeue @@ -389,10 +400,19 @@ func (r *NodeCIDRAllocationReconciler) updateNodeCIDRAllocationStatus(ctx contex } func (r *NodeCIDRAllocationReconciler) updateNodeTaints(ctx context.Context, nodes *corev1.NodeList) error { - ntc := &NodeTaintClient{} - for _, n := range nodes.Items { - ntc.Handle(&n) - if err := r.Client.Update(ctx, &n); err != nil { + ntc := taint.New() + for _, node := range nodes.Items { + // does not have an assigned PodCIDR - taint the Node + if node.Spec.PodCIDR == "" && !ntc.HasTaint(&node, nodeTaint.Key) { + ntc.AddNodeTaint(&node, nodeTaint) + } + + // has a PodCIDR assigned - remove the taint from the Node + if node.Spec.PodCIDR != "" && ntc.HasTaint(&node, nodeTaint.Key) { + ntc.RemoveNodeTaint(&node, nodeTaint.Key) + } + + if err := r.Client.Update(ctx, &node); err != nil { if apierrors.IsNotFound(err) { // node was removed after reconcile request was created - skip the Node continue @@ -413,7 +433,7 @@ func (r *NodeCIDRAllocationReconciler) triggerNodeCIDRAllocationReconcileFromNod usedByNodeCIDRAllocation := map[*v1alpha1.NodeCIDRAllocation]struct{}{} // implements a set-like structure to ensure that we only process a single reconcile for each unique match // get all the available NodeCIDRAllocations on the cluster - if err := r.Client.List(context.TODO(), allNodeCIDRAllocations, &client.ListOptions{ + if err := r.Client.List(ctx, allNodeCIDRAllocations, &client.ListOptions{ Namespace: corev1.NamespaceAll, }); err != nil { return []reconcile.Request{} @@ -454,7 +474,7 @@ func (r *NodeCIDRAllocationReconciler) SetupWithManager(mgr ctrl.Manager) error builder.WithPredicates(predicate.Funcs{ CreateFunc: func(_ event.CreateEvent) bool { return true }, UpdateFunc: func(_ event.UpdateEvent) bool { return false }, - DeleteFunc: func(_ event.DeleteEvent) bool { return false }, + DeleteFunc: func(_ event.DeleteEvent) bool { return true }, GenericFunc: func(_ event.GenericEvent) bool { return false }, }), ). diff --git a/internal/controller/taint.go b/internal/taint/taint.go similarity index 56% rename from internal/controller/taint.go rename to internal/taint/taint.go index cc40742..a126789 100644 --- a/internal/controller/taint.go +++ b/internal/taint/taint.go @@ -23,44 +23,51 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -package controller +package taint import ( corev1 "k8s.io/api/core/v1" ) -const ( - nodeTaintKey = "node.networking.statcan.gc.ca/network-unavailable" -) - +// NodeTainter is an interface that describes how to handle Node taints type NodeTainter interface { - Handle(*corev1.Node) + AddNodeTaint(*corev1.Node, corev1.Taint) + RemoveNodeTaint(*corev1.Node, string) + HasTaint(*corev1.Node, string) bool } +// NodeTaintClient provides a set of related functionality for managing Node taints for the CIDR-Allocator. +// +// This implementation is opinionated and designed for use with the CIDR-Allocator type NodeTaintClient struct{} +// A blank assignment to ensure conformity to the NodeTainer interface var _ NodeTainter = &NodeTaintClient{} -func (n *NodeTaintClient) Handle(node *corev1.Node) { - if node.Spec.PodCIDR == "" { - n.addNodeTaint(node) - } else { - n.removeNodeTaintIfExists(node) +// New creates a new NodeTaintClient and provides a reference to it +func New() *NodeTaintClient { + return &NodeTaintClient{} +} + +// taintExists checks the existing taints on the provided Node for one that has the key provided. If one exists, it returns true +func (n *NodeTaintClient) HasTaint(node *corev1.Node, key string) bool { + for _, t := range node.Spec.Taints { + if t.Key == key { + return true + } } + + return false } -func (n *NodeTaintClient) addNodeTaint(node *corev1.Node) { - node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{ - Key: nodeTaintKey, - Value: "true", - Effect: corev1.TaintEffectNoSchedule, - }) +func (n *NodeTaintClient) AddNodeTaint(node *corev1.Node, taint corev1.Taint) { + node.Spec.Taints = append(node.Spec.Taints, taint) } -func (n *NodeTaintClient) removeNodeTaintIfExists(node *corev1.Node) { +func (n *NodeTaintClient) RemoveNodeTaint(node *corev1.Node, key string) { taints := []corev1.Taint{} for _, taint := range node.Spec.Taints { - if taint.Key != nodeTaintKey { + if taint.Key != key { taints = append(taints, taint) } } diff --git a/internal/controller/taint_test.go b/internal/taint/taint_test.go similarity index 54% rename from internal/controller/taint_test.go rename to internal/taint/taint_test.go index b45aef9..fc2c171 100644 --- a/internal/controller/taint_test.go +++ b/internal/taint/taint_test.go @@ -23,47 +23,75 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -package controller_test +package taint_test import ( "testing" corev1 "k8s.io/api/core/v1" - "statcan.gc.ca/cidr-allocator/internal/controller" + "statcan.gc.ca/cidr-allocator/internal/taint" ) -func TestNodeTaintClientHandleNoPodCIDR(t *testing.T) { +const nodeTaintKey = "abc" + +func TestNodeTaintClientHasTaint(t *testing.T) { node := &corev1.Node{ Spec: corev1.NodeSpec{ - PodCIDR: "", + Taints: []corev1.Taint{ + { + Key: nodeTaintKey, + }, + }, }, } - ntc := &controller.NodeTaintClient{} - ntc.Handle(node) + ntc := taint.New() + + want := true + got := ntc.HasTaint(node, nodeTaintKey) + + if got != want { + t.Errorf("got %t, wanted %t", got, want) + } + + want = false + got = ntc.HasTaint(node, "xyz") + + if got != want { + t.Errorf("got %t, wanted %t", got, want) + } +} + +func TestNodeTaintClientAddNodeTaint(t *testing.T) { + node := &corev1.Node{ + Spec: corev1.NodeSpec{}, + } + + ntc := taint.New() + ntc.AddNodeTaint(node, corev1.Taint{Key: nodeTaintKey, Value: "true"}) - want := 1 - got := len(node.Spec.Taints) + want := true + got := len(node.Spec.Taints) == 1 && node.Spec.Taints[0].Key == nodeTaintKey if got != want { - t.Errorf("got %d, wanted %d", got, want) + t.Errorf("got %v, wanted %v", node.Spec.Taints, []corev1.Taint{{Key: nodeTaintKey, Value: "true"}}) } } -func TestNodeTaintClientHandlePodCIDRAssigned(t *testing.T) { +func TestNodeTaintClientRemoveNodeTaint(t *testing.T) { node := &corev1.Node{ Spec: corev1.NodeSpec{ - PodCIDR: "10.0.0.0/24", + Taints: []corev1.Taint{{Key: nodeTaintKey}}, }, } - ntc := &controller.NodeTaintClient{} - ntc.Handle(node) + ntc := taint.New() + ntc.RemoveNodeTaint(node, nodeTaintKey) - want := 0 - got := len(node.Spec.Taints) + want := true + got := len(node.Spec.Taints) == 0 if got != want { - t.Errorf("got %d, wanted %d", got, want) + t.Errorf("got %v, wanted %v", node.Spec.Taints, []corev1.Taint{}) } }