Skip to content

Commit

Permalink
Add funcs to add/delete finalizer on ClusterSlice
Browse files Browse the repository at this point in the history
- Added general functions to patch ClusterSlice ObjectMetadata
- Added functions to add/delete neg cleanup finalizer on ClusterSlice.
  This will allow to properly clean neg controllers when cluster slice
  is deleted
  • Loading branch information
panslava committed Dec 6, 2024
1 parent 38552ad commit 859835e
Show file tree
Hide file tree
Showing 4 changed files with 336 additions and 0 deletions.
53 changes: 53 additions & 0 deletions pkg/multiproject/finalizer/finalizer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package finalizer

import (
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterslice "k8s.io/ingress-gce/pkg/apis/clusterslice/v1"
clustersliceclient "k8s.io/ingress-gce/pkg/clusterslice/client/clientset/versioned/typed/clusterslice/v1"
"k8s.io/ingress-gce/pkg/utils/patch"
"k8s.io/ingress-gce/pkg/utils/slice"
"k8s.io/klog/v2"
)

const (
ClusterSliceNEGCleanupFinalizer = "multiproject.networking.gke.io/neg-cleanup"
)

func EnsureClusterSliceNEGCleanupFinalizer(cs *clusterslice.ClusterSlice, csClient clustersliceclient.ClusterSliceInterface, logger klog.Logger) error {
return ensureClusterSliceFinalizer(cs, ClusterSliceNEGCleanupFinalizer, csClient, logger)
}

func DeleteClusterSliceNEGCleanupFinalizer(cs *clusterslice.ClusterSlice, csClient clustersliceclient.ClusterSliceInterface, logger klog.Logger) error {
return deleteClusterSliceFinalizer(cs, ClusterSliceNEGCleanupFinalizer, csClient, logger)
}

func ensureClusterSliceFinalizer(cs *clusterslice.ClusterSlice, key string, csClient clustersliceclient.ClusterSliceInterface, logger klog.Logger) error {
if HasGivenFinalizer(cs.ObjectMeta, key) {
return nil
}

// Make a deep copy of the ClusterSlice to avoid mutating the shared informer cache.
updatedObjectMeta := cs.ObjectMeta.DeepCopy()
updatedObjectMeta.Finalizers = append(updatedObjectMeta.Finalizers, key)

logger.V(2).Info("Adding finalizer to ClusterSlice", "finalizerKey", key, "clusterSlice", fmt.Sprintf("%s/%s", cs.Namespace, cs.Name))
return patch.PatchClusterSliceObjectMetadata(csClient, cs, *updatedObjectMeta)
}

func deleteClusterSliceFinalizer(cs *clusterslice.ClusterSlice, key string, csClient clustersliceclient.ClusterSliceInterface, logger klog.Logger) error {
if !HasGivenFinalizer(cs.ObjectMeta, key) {
return nil
}

updatedObjectMeta := cs.ObjectMeta.DeepCopy()
updatedObjectMeta.Finalizers = slice.RemoveString(updatedObjectMeta.Finalizers, key, nil)
logger.V(2).Info("Deleting finalizer from ClusterSlice", "finalizerKey", key, "clusterSlice", fmt.Sprintf("%s/%s", cs.Namespace, cs.Name))
return patch.PatchClusterSliceObjectMetadata(csClient, cs, *updatedObjectMeta)
}

// HasGivenFinalizer is true if the passed in meta has the specified finalizer.
func HasGivenFinalizer(m metav1.ObjectMeta, key string) bool {
return slice.ContainsString(m.Finalizers, key, nil)
}
142 changes: 142 additions & 0 deletions pkg/multiproject/finalizer/finalizer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
package finalizer

import (
"context"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterslicev1 "k8s.io/ingress-gce/pkg/apis/clusterslice/v1"
clusterslicefake "k8s.io/ingress-gce/pkg/clusterslice/client/clientset/versioned/fake"
"k8s.io/ingress-gce/pkg/utils/slice"
"k8s.io/klog/v2/ktesting"
)

func TestEnsureClusterSliceNEGCleanupFinalizer(t *testing.T) {
testCases := []struct {
desc string
cs *clusterslicev1.ClusterSlice
expectedFinalizerPresent bool
}{
{
desc: "Finalizer not present, should add it",
cs: &clusterslicev1.ClusterSlice{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cs-add-finalizer",
Namespace: "default",
Finalizers: []string{},
},
},
expectedFinalizerPresent: true,
},
{
desc: "Finalizer already present, should remain",
cs: &clusterslicev1.ClusterSlice{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cs-finalizer-present",
Namespace: "default",
Finalizers: []string{
ClusterSliceNEGCleanupFinalizer,
},
},
},
expectedFinalizerPresent: true,
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
csClient := clusterslicefake.NewSimpleClientset(tc.cs)
csInterface := csClient.FlagsV1().ClusterSlices(tc.cs.Namespace)

// Create the initial ClusterSlice object
_, err := csInterface.Create(context.TODO(), tc.cs, metav1.CreateOptions{})
if err != nil {
t.Fatalf("Failed to create ClusterSlice: %v", err)
}

logger, _ := ktesting.NewTestContext(t)

err = EnsureClusterSliceNEGCleanupFinalizer(tc.cs, csInterface, logger)
if err != nil {
t.Fatalf("EnsureClusterSliceNEGCleanupFinalizer returned error: %v", err)
}

// Retrieve the updated ClusterSlice
updatedCS, err := csInterface.Get(context.TODO(), tc.cs.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get updated ClusterSlice: %v", err)
}

// Check if the finalizer is present as expected
hasFinalizer := slice.ContainsString(updatedCS.Finalizers, ClusterSliceNEGCleanupFinalizer, nil)
if hasFinalizer != tc.expectedFinalizerPresent {
t.Errorf("Finalizer presence mismatch: expected %v, got %v", tc.expectedFinalizerPresent, hasFinalizer)
}
})
}
}

func TestEnsureDeleteClusterSliceNEGCleanupFinalizer(t *testing.T) {
testCases := []struct {
desc string
cs *clusterslicev1.ClusterSlice
expectedFinalizerPresent bool
}{
{
desc: "Finalizer present, should be removed",
cs: &clusterslicev1.ClusterSlice{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cs-remove-finalizer",
Namespace: "default",
Finalizers: []string{
ClusterSliceNEGCleanupFinalizer,
},
},
},
expectedFinalizerPresent: false,
},
{
desc: "Finalizer not present, remains absent",
cs: &clusterslicev1.ClusterSlice{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cs-finalizer-not-present",
Namespace: "default",
Finalizers: []string{},
},
},
expectedFinalizerPresent: false,
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
csClient := clusterslicefake.NewSimpleClientset(tc.cs)
csInterface := csClient.FlagsV1().ClusterSlices(tc.cs.Namespace)

// Create the initial ClusterSlice object
_, err := csInterface.Create(context.TODO(), tc.cs, metav1.CreateOptions{})
if err != nil {
t.Fatalf("Failed to create ClusterSlice: %v", err)
}

logger, _ := ktesting.NewTestContext(t)

err = DeleteClusterSliceNEGCleanupFinalizer(tc.cs, csInterface, logger)
if err != nil {
t.Fatalf("DeleteClusterSliceNEGCleanupFinalizer returned error: %v", err)
}

// Retrieve the updated ClusterSlice
updatedCS, err := csInterface.Get(context.TODO(), tc.cs.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get updated ClusterSlice: %v", err)
}

// Check if the finalizer is present as expected
hasFinalizer := slice.ContainsString(updatedCS.Finalizers, ClusterSliceNEGCleanupFinalizer, nil)
if hasFinalizer != tc.expectedFinalizerPresent {
t.Errorf("Finalizer presence mismatch: expected %v, got %v", tc.expectedFinalizerPresent, hasFinalizer)
}
})
}
}
41 changes: 41 additions & 0 deletions pkg/utils/patch/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,19 @@ limitations under the License.
package patch

import (
"context"
"encoding/json"
"fmt"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/jsonmergepatch"
"k8s.io/apimachinery/pkg/util/strategicpatch"
coreclient "k8s.io/client-go/kubernetes/typed/core/v1"
svchelpers "k8s.io/cloud-provider/service/helpers"
clusterslice "k8s.io/ingress-gce/pkg/apis/clusterslice/v1"
clustersliceclient "k8s.io/ingress-gce/pkg/clusterslice/client/clientset/versioned/typed/clusterslice/v1"
)

// StrategicMergePatchBytes returns a patch between the old and new object using a strategic merge patch.
Expand Down Expand Up @@ -84,3 +88,40 @@ func PatchServiceLoadBalancerStatus(client coreclient.CoreV1Interface, svc *core
_, err := svchelpers.PatchService(client, svc, newSvc)
return err
}

// PatchClusterSliceObjectMetadata patches the given ClusterSlice's metadata based on new metadata.
func PatchClusterSliceObjectMetadata(client clustersliceclient.ClusterSliceInterface, cs *clusterslice.ClusterSlice, newObjectMetadata metav1.ObjectMeta) error {
// Reset Spec to ensure only ObjectMeta is patched.
newCS := cs.DeepCopy()
newCS.Spec = cs.Spec

newCS.ObjectMeta = newObjectMetadata

patchBytes, err := getClusterSlicePatchBytes(cs, newCS)
if err != nil {
return err
}

_, err = client.Patch(context.Background(), newCS.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{})
return err
}

// getClusterSlicePatchBytes generates the patch bytes for updating a ClusterSlice.
func getClusterSlicePatchBytes(oldCS, newCS *clusterslice.ClusterSlice) ([]byte, error) {
oldData, err := json.Marshal(oldCS)
if err != nil {
return nil, fmt.Errorf("failed to marshal old object: %v", err)
}

newData, err := json.Marshal(newCS)
if err != nil {
return nil, fmt.Errorf("failed to marshal new object: %v", err)
}

patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, clusterslice.ClusterSlice{})
if err != nil {
return nil, fmt.Errorf("failed to create patch: %v", err)
}

return patchBytes, nil
}
100 changes: 100 additions & 0 deletions pkg/utils/patch/patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import (
v1 "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
clusterslice "k8s.io/ingress-gce/pkg/apis/clusterslice/v1"
clusterslicefake "k8s.io/ingress-gce/pkg/clusterslice/client/clientset/versioned/fake"

"k8s.io/ingress-gce/pkg/utils/slice"
)

Expand Down Expand Up @@ -250,6 +253,103 @@ func TestPatchServiceLoadBalancerStatus(t *testing.T) {
}
}

func TestPatchClusterSliceObjectMetadata(t *testing.T) {
for _, tc := range []struct {
desc string
cs *clusterslice.ClusterSlice
updateObjectMetaFunc func(*clusterslice.ClusterSlice) *clusterslice.ClusterSlice
}{
{
desc: "add annotation",
cs: &clusterslice.ClusterSlice{ObjectMeta: metav1.ObjectMeta{Name: "add-annotation-cs", Namespace: "ns1"}},
updateObjectMetaFunc: func(cs *clusterslice.ClusterSlice) *clusterslice.ClusterSlice {
ret := cs.DeepCopy()
if ret.Annotations == nil {
ret.Annotations = make(map[string]string)
}
ret.Annotations["test-annotation-key3"] = "test-value3"
return ret
},
},
{
desc: "delete annotation",
cs: &clusterslice.ClusterSlice{ObjectMeta: metav1.ObjectMeta{Name: "delete-annotation-cs", Namespace: "ns2"}},
updateObjectMetaFunc: func(cs *clusterslice.ClusterSlice) *clusterslice.ClusterSlice {
ret := cs.DeepCopy()
delete(ret.Annotations, testAnnotationKey)
return ret
},
},
{
desc: "delete all annotations",
cs: &clusterslice.ClusterSlice{ObjectMeta: metav1.ObjectMeta{Name: "delete-all-annotations-cs", Namespace: "ns3"}},
updateObjectMetaFunc: func(cs *clusterslice.ClusterSlice) *clusterslice.ClusterSlice {
ret := cs.DeepCopy()
ret.Annotations = nil
return ret
},
},
{
desc: "add finalizer",
cs: &clusterslice.ClusterSlice{ObjectMeta: metav1.ObjectMeta{Name: "add-finalizer-cs", Namespace: "ns4"}},
updateObjectMetaFunc: func(cs *clusterslice.ClusterSlice) *clusterslice.ClusterSlice {
ret := cs.DeepCopy()
ret.Finalizers = append(ret.Finalizers, "new-test-finalizer")
return ret
},
},
{
desc: "delete finalizer",
cs: &clusterslice.ClusterSlice{ObjectMeta: metav1.ObjectMeta{Name: "delete-finalizer-cs", Namespace: "ns5"}},
updateObjectMetaFunc: func(cs *clusterslice.ClusterSlice) *clusterslice.ClusterSlice {
ret := cs.DeepCopy()
ret.Finalizers = slice.RemoveString(ret.Finalizers, testFinalizer, nil)
return ret
},
},
{
desc: "delete all finalizers",
cs: &clusterslice.ClusterSlice{ObjectMeta: metav1.ObjectMeta{Name: "delete-all-finalizers-cs", Namespace: "ns6"}},
updateObjectMetaFunc: func(cs *clusterslice.ClusterSlice) *clusterslice.ClusterSlice {
ret := cs.DeepCopy()
ret.Finalizers = nil
return ret
},
},
{
desc: "delete both annotation and finalizer",
cs: &clusterslice.ClusterSlice{ObjectMeta: metav1.ObjectMeta{Name: "delete-annotation-and-finalizer-cs", Namespace: "ns7"}},
updateObjectMetaFunc: func(cs *clusterslice.ClusterSlice) *clusterslice.ClusterSlice {
ret := cs.DeepCopy()
ret.Finalizers = slice.RemoveString(ret.Finalizers, testFinalizer, nil)
delete(ret.Annotations, testAnnotationKey)
return ret
},
},
} {
t.Run(tc.desc, func(t *testing.T) {
csKey := fmt.Sprintf("%s/%s", tc.cs.Namespace, tc.cs.Name)
csClient := clusterslicefake.NewSimpleClientset().FlagsV1().ClusterSlices(tc.cs.Namespace)
if _, err := csClient.Create(context.TODO(), tc.cs, metav1.CreateOptions{}); err != nil {
t.Fatalf("Create(%s) = %v, want nil", csKey, err)
}
expectCS := tc.updateObjectMetaFunc(tc.cs)
err := PatchClusterSliceObjectMetadata(csClient, tc.cs, expectCS.ObjectMeta)
if err != nil {
t.Fatalf("PatchClusterSliceObjectMetadata(%s) = %v, want nil", csKey, err)
}

gotCS, err := csClient.Get(context.TODO(), tc.cs.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Get(%s) = %v, want nil", csKey, err)
}
if diff := cmp.Diff(expectCS.ObjectMeta, gotCS.ObjectMeta); diff != "" {
t.Errorf("Got mismatch for ClusterSlice ObjectMeta (-want +got):\n%s", diff)
}
})
}
}

const (
testAnnotationKey = "test-annotations-key1"
testFinalizer = "test-finalizer"
Expand Down

0 comments on commit 859835e

Please sign in to comment.