diff --git a/.github/workflows/continuous-delivery.yml b/.github/workflows/continuous-delivery.yml index 54aa4175e1..f78d21225a 100644 --- a/.github/workflows/continuous-delivery.yml +++ b/.github/workflows/continuous-delivery.yml @@ -363,8 +363,7 @@ jobs: flavor: | suffix=-ubi8 tags: | - type=ref,event=branch - type=ref,event=pr + type=raw,value=${{ steps.build-meta.outputs.tag_name }} - name: Set up QEMU uses: docker/setup-qemu-action@v3 @@ -1851,7 +1850,7 @@ jobs: e2e-openshift: name: Run E2E on OpenShift - if: | + if: | always() && !cancelled() && vars.OPENSHIFT_ENABLED == 'true' && needs.generate-jobs.outputs.openshiftEnabled == 'true' && diff --git a/api/v1/cluster_types.go b/api/v1/cluster_types.go index 1d3172a90a..771c03e647 100644 --- a/api/v1/cluster_types.go +++ b/api/v1/cluster_types.go @@ -2998,7 +2998,7 @@ func (cluster *Cluster) IsInstanceFenced(instance string) bool { return false } - if fencedInstances.Has(utils.FenceAllServers) { + if fencedInstances.Has(utils.FenceAllInstances) { return true } return fencedInstances.Has(instance) diff --git a/controllers/cluster_restore.go b/controllers/cluster_restore.go index 2a77dcffed..d841fcb8c1 100644 --- a/controllers/cluster_restore.go +++ b/controllers/cluster_restore.go @@ -104,22 +104,10 @@ func ensureClusterIsNotFenced( cli client.Client, cluster *apiv1.Cluster, ) error { - fencedInstances, err := utils.GetFencedInstances(cluster.Annotations) - if err != nil { - return err - } - if fencedInstances.Len() == 0 { - return nil - } - - clusterOrig := cluster.DeepCopy() - - // we remove the fenced instances this way to ensure that the patch method will work - if err := utils.RemoveFencedInstance(utils.FenceAllServers, &cluster.ObjectMeta); err != nil { - return err - } - - return cli.Patch(ctx, cluster, client.MergeFrom(clusterOrig)) + return utils.NewFencingMetadataExecutor(cli). + RemoveFencing(). + ForAllInstances(). + Execute(ctx, client.ObjectKeyFromObject(cluster), cluster) } // restoreClusterStatus bootstraps the status needed to make the restored cluster work diff --git a/controllers/cluster_restore_test.go b/controllers/cluster_restore_test.go index cfc168412d..907c83c1a7 100644 --- a/controllers/cluster_restore_test.go +++ b/controllers/cluster_restore_test.go @@ -81,8 +81,9 @@ var _ = Describe("ensureClusterIsNotFenced", func() { Context("when fenced instances exist", func() { BeforeEach(func() { - err := utils.AddFencedInstance(utils.FenceAllServers, &cluster.ObjectMeta) + modified, err := utils.AddFencedInstance(utils.FenceAllInstances, &cluster.ObjectMeta) Expect(err).ToNot(HaveOccurred()) + Expect(modified).To(BeTrue()) mockCli = fake.NewClientBuilder(). WithScheme(k8scheme.BuildWithAllKnownScheme()). WithObjects(cluster). diff --git a/internal/cmd/plugin/fence/fence.go b/internal/cmd/plugin/fence/fence.go index 6359593e91..33c4a1165a 100644 --- a/internal/cmd/plugin/fence/fence.go +++ b/internal/cmd/plugin/fence/fence.go @@ -21,14 +21,22 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/types" + + apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/cloudnative-pg/cloudnative-pg/internal/cmd/plugin" - "github.com/cloudnative-pg/cloudnative-pg/pkg/resources" "github.com/cloudnative-pg/cloudnative-pg/pkg/utils" ) // fencingOn marks an instance in a cluster as fenced func fencingOn(ctx context.Context, clusterName string, serverName string) error { - err := resources.ApplyFenceFunc(ctx, plugin.Client, clusterName, plugin.Namespace, serverName, utils.AddFencedInstance) + err := utils.NewFencingMetadataExecutor(plugin.Client). + AddFencing(). + ForInstance(serverName). + Execute(ctx, + types.NamespacedName{Name: clusterName, Namespace: plugin.Namespace}, + &apiv1.Cluster{}, + ) if err != nil { return err } @@ -38,8 +46,13 @@ func fencingOn(ctx context.Context, clusterName string, serverName string) error // fencingOff marks an instance in a cluster as not fenced func fencingOff(ctx context.Context, clusterName string, serverName string) error { - err := resources.ApplyFenceFunc(ctx, plugin.Client, clusterName, plugin.Namespace, - serverName, utils.RemoveFencedInstance) + err := utils.NewFencingMetadataExecutor(plugin.Client). + RemoveFencing(). + ForInstance(serverName). + Execute(ctx, + types.NamespacedName{Name: clusterName, Namespace: plugin.Namespace}, + &apiv1.Cluster{}, + ) if err != nil { return err } diff --git a/internal/cmd/plugin/hibernate/on.go b/internal/cmd/plugin/hibernate/on.go index 412b4ceb38..640661a578 100644 --- a/internal/cmd/plugin/hibernate/on.go +++ b/internal/cmd/plugin/hibernate/on.go @@ -177,14 +177,14 @@ func (on *onCommand) fenceClusterStep() error { contextLogger := log.FromContext(on.ctx) contextLogger.Debug("applying the fencing annotation to the cluster manifest") - if err := resources.ApplyFenceFunc( - on.ctx, - plugin.Client, - on.cluster.Name, - plugin.Namespace, - utils.FenceAllServers, - utils.AddFencedInstance, - ); err != nil { + if err := utils.NewFencingMetadataExecutor(plugin.Client). + AddFencing(). + ForAllInstances(). + Execute( + on.ctx, + types.NamespacedName{Name: on.cluster.Name, Namespace: plugin.Namespace}, + &apiv1.Cluster{}, + ); err != nil { return err } contextLogger.Debug("fencing annotation set on the cluster manifest") @@ -202,15 +202,11 @@ func (on *onCommand) rollbackFenceClusterIfNeeded() { contextLogger := log.FromContext(on.ctx) fmt.Println("rolling back hibernation: removing the fencing annotation") - err := resources.ApplyFenceFunc( - on.ctx, - plugin.Client, - on.cluster.Name, - plugin.Namespace, - utils.FenceAllServers, - utils.RemoveFencedInstance, - ) - if err != nil { + if err := utils.NewFencingMetadataExecutor(plugin.Client). + RemoveFencing(). + ForAllInstances(). + Execute(on.ctx, + types.NamespacedName{Name: on.cluster.Name, Namespace: plugin.Namespace}, &apiv1.Cluster{}); err != nil { contextLogger.Error(err, "Rolling back from hibernation failed") } } diff --git a/internal/cmd/plugin/status/status.go b/internal/cmd/plugin/status/status.go index 47ef3d6560..1fe84c2f7d 100644 --- a/internal/cmd/plugin/status/status.go +++ b/internal/cmd/plugin/status/status.go @@ -158,7 +158,7 @@ func ExtractPostgresqlStatus(ctx context.Context, clusterName string) (*Postgres } func listFencedInstances(fencedInstances *stringset.Data) string { - if fencedInstances.Has(utils.FenceAllServers) { + if fencedInstances.Has(utils.FenceAllInstances) { return "All Instances" } return strings.Join(fencedInstances.ToList(), ", ") diff --git a/pkg/reconciler/backup/volumesnapshot/offline.go b/pkg/reconciler/backup/volumesnapshot/offline.go index 87ebc36fc2..1ce514c121 100644 --- a/pkg/reconciler/backup/volumesnapshot/offline.go +++ b/pkg/reconciler/backup/volumesnapshot/offline.go @@ -31,7 +31,6 @@ import ( apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/cloudnative-pg/cloudnative-pg/pkg/management/log" - "github.com/cloudnative-pg/cloudnative-pg/pkg/resources" "github.com/cloudnative-pg/cloudnative-pg/pkg/utils" ) @@ -124,19 +123,10 @@ func (o *offlineExecutor) ensurePodIsFenced( "targetBackup", backup.Name, "targetPod", targetPodName, ) } - - err = resources.ApplyFenceFunc( - ctx, - o.cli, - cluster.Name, - cluster.Namespace, - targetPodName, - utils.AddFencedInstance, - ) - if errors.Is(err, utils.ErrorServerAlreadyFenced) { - return nil - } - if err != nil { + if err = utils.NewFencingMetadataExecutor(o.cli). + AddFencing(). + ForInstance(targetPodName). + Execute(ctx, client.ObjectKeyFromObject(cluster), cluster); err != nil { return err } diff --git a/pkg/reconciler/backup/volumesnapshot/offline_test.go b/pkg/reconciler/backup/volumesnapshot/offline_test.go index 4f4c6edb0a..ac0175ab7c 100644 --- a/pkg/reconciler/backup/volumesnapshot/offline_test.go +++ b/pkg/reconciler/backup/volumesnapshot/offline_test.go @@ -115,8 +115,9 @@ var _ = Describe("offlineExecutor", func() { }) It("finalize should remove the fencing annotation from the cluster", func(ctx SpecContext) { - err := utils.AddFencedInstance(pod.Name, &cluster.ObjectMeta) + modified, err := utils.AddFencedInstance(pod.Name, &cluster.ObjectMeta) Expect(err).ToNot(HaveOccurred()) + Expect(modified).To(BeTrue()) err = oe.cli.Update(ctx, cluster) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/reconciler/backup/volumesnapshot/reconciler.go b/pkg/reconciler/backup/volumesnapshot/reconciler.go index 935a9bb531..152e9212f2 100644 --- a/pkg/reconciler/backup/volumesnapshot/reconciler.go +++ b/pkg/reconciler/backup/volumesnapshot/reconciler.go @@ -20,7 +20,6 @@ import ( "context" "encoding/base64" "encoding/json" - "errors" "fmt" "strconv" "time" @@ -37,7 +36,6 @@ import ( "github.com/cloudnative-pg/cloudnative-pg/pkg/management/log" "github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres" "github.com/cloudnative-pg/cloudnative-pg/pkg/reconciler/persistentvolumeclaim" - "github.com/cloudnative-pg/cloudnative-pg/pkg/resources" "github.com/cloudnative-pg/cloudnative-pg/pkg/resources/instance" "github.com/cloudnative-pg/cloudnative-pg/pkg/utils" ) @@ -346,18 +344,10 @@ func EnsurePodIsUnfenced( ) error { contextLogger := log.FromContext(ctx) - err := resources.ApplyFenceFunc( - ctx, - cli, - cluster.Name, - cluster.Namespace, - targetPod.Name, - utils.RemoveFencedInstance, - ) - if errors.Is(err, utils.ErrorServerAlreadyUnfenced) { - return nil - } - if err != nil { + if err := utils.NewFencingMetadataExecutor(cli). + RemoveFencing(). + ForInstance(targetPod.Name). + Execute(ctx, client.ObjectKeyFromObject(cluster), cluster); err != nil { return err } diff --git a/pkg/resources/fencing.go b/pkg/resources/fencing.go deleted file mode 100644 index 2e97265c14..0000000000 --- a/pkg/resources/fencing.go +++ /dev/null @@ -1,68 +0,0 @@ -/* -Copyright The CloudNativePG Contributors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package resources - -import ( - "context" - "fmt" - - corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" - "github.com/cloudnative-pg/cloudnative-pg/pkg/utils" -) - -// ApplyFenceFunc applies a given fencing function to a cluster in a namespace -func ApplyFenceFunc( - ctx context.Context, - cli client.Client, - clusterName string, - namespace string, - serverName string, - fenceFunc func(string, *v1.ObjectMeta) error, -) error { - var cluster apiv1.Cluster - - // Get the Cluster object - err := cli.Get(ctx, client.ObjectKey{Namespace: namespace, Name: clusterName}, &cluster) - if err != nil { - return err - } - - if serverName != utils.FenceAllServers { - // Check if the Pod exist - var pod corev1.Pod - err = cli.Get(ctx, client.ObjectKey{Namespace: namespace, Name: serverName}, &pod) - if err != nil { - return fmt.Errorf("node %s not found in namespace %s", serverName, namespace) - } - } - - fencedCluster := cluster.DeepCopy() - if err = fenceFunc(serverName, &fencedCluster.ObjectMeta); err != nil { - return err - } - - err = cli.Patch(ctx, fencedCluster, client.MergeFrom(&cluster)) - if err != nil { - return err - } - - return nil -} diff --git a/pkg/utils/fencing.go b/pkg/utils/fencing.go index 17af844093..7ead694e7a 100644 --- a/pkg/utils/fencing.go +++ b/pkg/utils/fencing.go @@ -17,11 +17,16 @@ limitations under the License. package utils import ( + "context" "encoding/json" "errors" + "fmt" "sort" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/cloudnative-pg/cloudnative-pg/pkg/stringset" ) @@ -31,23 +36,15 @@ var ( // have an invalid syntax ErrorFencedInstancesSyntax = errors.New("fencedInstances annotation has invalid syntax") - // ErrorServerAlreadyFenced is emitted when trying to fence an instance - // which is already fenced - ErrorServerAlreadyFenced = errors.New("this instance has already been fenced") - - // ErrorServerAlreadyUnfenced is emitted when trying to unfencing an instance - // which was not fenced - ErrorServerAlreadyUnfenced = errors.New("this instance was not fenced") - // ErrorSingleInstanceUnfencing is emitted when unfencing a single instance // while all the cluster is fenced ErrorSingleInstanceUnfencing = errors.New("unfencing an instance while the whole cluster is fenced is not supported") ) const ( - // FenceAllServers is the wildcard that, if put inside the fenced instances list, will fence every + // FenceAllInstances is the wildcard that, if put inside the fenced instances list, will fence every // CNPG instance - FenceAllServers = "*" + FenceAllInstances = "*" ) // GetFencedInstances gets the set of fenced servers from the annotations @@ -58,18 +55,21 @@ func GetFencedInstances(annotations map[string]string) (*stringset.Data, error) } var fencedInstancesList []string - err := json.Unmarshal([]byte(fencedInstances), &fencedInstancesList) - if err != nil { + if err := json.Unmarshal([]byte(fencedInstances), &fencedInstancesList); err != nil { return nil, ErrorFencedInstancesSyntax } return stringset.From(fencedInstancesList), nil } -// SetFencedInstances sets the list of fenced servers inside the annotations -func SetFencedInstances(object *metav1.ObjectMeta, data *stringset.Data) error { +// setFencedInstances sets the list of fenced servers inside the annotations +func setFencedInstances(object metav1.Object, data *stringset.Data) error { + annotations := object.GetAnnotations() + defer func() { + object.SetAnnotations(annotations) + }() if data.Len() == 0 { - delete(object.Annotations, FencedInstanceAnnotation) + delete(annotations, FencedInstanceAnnotation) return nil } @@ -80,58 +80,126 @@ func SetFencedInstances(object *metav1.ObjectMeta, data *stringset.Data) error { if err != nil { return err } - if object.Annotations == nil { - object.Annotations = make(map[string]string) + if annotations == nil { + annotations = make(map[string]string) } - object.Annotations[FencedInstanceAnnotation] = string(annotationValue) + annotations[FencedInstanceAnnotation] = string(annotationValue) return nil } // AddFencedInstance adds the given server name to the FencedInstanceAnnotation annotation // returns an error if the instance was already fenced -func AddFencedInstance(serverName string, object *metav1.ObjectMeta) error { - fencedInstances, err := GetFencedInstances(object.Annotations) +func AddFencedInstance(instanceName string, object metav1.Object) (bool, error) { + fencedInstances, err := GetFencedInstances(object.GetAnnotations()) if err != nil { - return err + return false, err } - if fencedInstances.Has(FenceAllServers) { - return nil - } - if fencedInstances.Has(serverName) { - return ErrorServerAlreadyFenced + if fencedInstances.Has(FenceAllInstances) || fencedInstances.Has(instanceName) { + return false, nil } - if serverName == FenceAllServers { - fencedInstances = stringset.From([]string{FenceAllServers}) - } else { - fencedInstances.Put(serverName) + switch instanceName { + case FenceAllInstances: + fencedInstances = stringset.From([]string{FenceAllInstances}) + default: + fencedInstances.Put(instanceName) } - return SetFencedInstances(object, fencedInstances) + return true, setFencedInstances(object, fencedInstances) } -// RemoveFencedInstance removes the given server name from the FencedInstanceAnnotation annotation +// removeFencedInstance removes the given server name from the FencedInstanceAnnotation annotation // returns an error if the instance was already unfenced -func RemoveFencedInstance(serverName string, object *metav1.ObjectMeta) error { - if serverName == FenceAllServers { - return SetFencedInstances(object, stringset.New()) +func removeFencedInstance(instanceName string, object metav1.Object) (bool, error) { + fencedInstances, err := GetFencedInstances(object.GetAnnotations()) + if err != nil { + return false, err + } + if fencedInstances.Len() == 0 { + return false, nil + } + if instanceName == FenceAllInstances { + return true, setFencedInstances(object, stringset.New()) } - fencedInstances, err := GetFencedInstances(object.Annotations) - if err != nil { + if fencedInstances.Has(FenceAllInstances) { + return false, ErrorSingleInstanceUnfencing + } + + if !fencedInstances.Has(instanceName) { + return false, nil + } + + fencedInstances.Delete(instanceName) + return true, setFencedInstances(object, fencedInstances) +} + +// FencingMetadataExecutor executes the logic regarding adding and removing the fencing annotation for a kubernetes +// object +type FencingMetadataExecutor struct { + fenceFunc func(string, metav1.Object) (appliedChange bool, err error) + instanceName string + cli client.Client +} + +// NewFencingMetadataExecutor creates a fluent client for FencingMetadataExecutor +func NewFencingMetadataExecutor(cli client.Client) *FencingMetadataExecutor { + return &FencingMetadataExecutor{ + cli: cli, + } +} + +// AddFencing instructs the client to execute the logic of adding a instance +func (fb *FencingMetadataExecutor) AddFencing() *FencingMetadataExecutor { + fb.fenceFunc = AddFencedInstance + return fb +} + +// RemoveFencing instructs the client to execute the logic of removing an instance +func (fb *FencingMetadataExecutor) RemoveFencing() *FencingMetadataExecutor { + fb.fenceFunc = removeFencedInstance + return fb +} + +// ForAllInstances applies the logic to all cluster instances +func (fb *FencingMetadataExecutor) ForAllInstances() *FencingMetadataExecutor { + fb.instanceName = FenceAllInstances + return fb +} + +// ForInstance applies the logic to the specified instance +func (fb *FencingMetadataExecutor) ForInstance(instanceName string) *FencingMetadataExecutor { + fb.instanceName = instanceName + return fb +} + +// Execute executes the instructions given with the fluent builder, returns any error encountered +func (fb *FencingMetadataExecutor) Execute(ctx context.Context, key types.NamespacedName, obj client.Object) error { + if fb.instanceName == "" { + return errors.New("chose an operation to execute") + } + + if err := fb.cli.Get(ctx, key, obj); err != nil { return err } - if fencedInstances.Has(FenceAllServers) { - return ErrorSingleInstanceUnfencing + if fb.instanceName != FenceAllInstances { + var pod corev1.Pod + if err := fb.cli.Get(ctx, client.ObjectKey{Namespace: key.Namespace, Name: fb.instanceName}, &pod); err != nil { + return fmt.Errorf("node %s not found in namespace %s", fb.instanceName, key.Namespace) + } } - if !fencedInstances.Has(serverName) { - return ErrorServerAlreadyUnfenced + fencedObject := obj.DeepCopyObject().(client.Object) + appliedChange, err := fb.fenceFunc(fb.instanceName, fencedObject) + if err != nil { + return err + } + if !appliedChange { + return nil } - fencedInstances.Delete(serverName) - return SetFencedInstances(object, fencedInstances) + return fb.cli.Patch(ctx, fencedObject, client.MergeFrom(obj)) } diff --git a/pkg/utils/fencing_test.go b/pkg/utils/fencing_test.go index db047bb50c..27b1043cca 100644 --- a/pkg/utils/fencing_test.go +++ b/pkg/utils/fencing_test.go @@ -39,8 +39,9 @@ var _ = Describe("Fencing annotation handling", func() { FencedInstanceAnnotation: jsonMarshal("cluster-example-1"), }, } - err := RemoveFencedInstance("cluster-example-1", &clusterMeta) + modified, err := removeFencedInstance("cluster-example-1", &clusterMeta) Expect(err).NotTo(HaveOccurred()) + Expect(modified).To(BeTrue()) Expect(clusterMeta.Annotations).NotTo(HaveKey(FencedInstanceAnnotation)) }) It("should correctly remove only that instance when unfenced", func() { @@ -49,18 +50,24 @@ var _ = Describe("Fencing annotation handling", func() { FencedInstanceAnnotation: jsonMarshal("cluster-example-1", "cluster-example-2"), }, } - err := RemoveFencedInstance("cluster-example-1", &clusterMeta) + modified, err := removeFencedInstance("cluster-example-1", &clusterMeta) Expect(err).NotTo(HaveOccurred()) - Expect(clusterMeta.Annotations).To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-2"))) + Expect(modified).To(BeTrue()) + Expect(clusterMeta.Annotations). + ToNot(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-1"))) + Expect(clusterMeta.Annotations). + To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-2"))) }) - It("should return an error if tried to fence again", func() { + + It("should not return an error if tried to fence again", func() { clusterMeta := metav1.ObjectMeta{ Annotations: map[string]string{ FencedInstanceAnnotation: jsonMarshal("cluster-example-1", "cluster-example-2"), }, } - err := AddFencedInstance("cluster-example-1", &clusterMeta) - Expect(err).To(HaveOccurred()) + modified, err := AddFencedInstance("cluster-example-1", &clusterMeta) + Expect(err).ToNot(HaveOccurred()) + Expect(modified).To(BeFalse()) Expect(clusterMeta.Annotations). To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-1", "cluster-example-2"))) }) @@ -70,9 +77,11 @@ var _ = Describe("Fencing annotation handling", func() { clusterMeta := metav1.ObjectMeta{ Annotations: map[string]string{}, } - err := AddFencedInstance("cluster-example-1", &clusterMeta) + modified, err := AddFencedInstance("cluster-example-1", &clusterMeta) Expect(err).NotTo(HaveOccurred()) - Expect(clusterMeta.Annotations).To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-1"))) + Expect(modified).To(BeTrue()) + Expect(clusterMeta.Annotations). + To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-1"))) }) It("should correctly add it to other fenced instances", func() { clusterMeta := metav1.ObjectMeta{ @@ -80,19 +89,21 @@ var _ = Describe("Fencing annotation handling", func() { FencedInstanceAnnotation: jsonMarshal("cluster-example-2"), }, } - err := AddFencedInstance("cluster-example-1", &clusterMeta) + modified, err := AddFencedInstance("cluster-example-1", &clusterMeta) Expect(err).NotTo(HaveOccurred()) + Expect(modified).To(BeTrue()) Expect(clusterMeta.Annotations). To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-1", "cluster-example-2"))) }) - It("should return an error if tried to unfence again", func() { + It("should not return an error if tried to unfence again", func() { clusterMeta := metav1.ObjectMeta{ Annotations: map[string]string{ FencedInstanceAnnotation: jsonMarshal("cluster-example-2"), }, } - err := RemoveFencedInstance("cluster-example-1", &clusterMeta) - Expect(err).To(HaveOccurred()) + modified, err := removeFencedInstance("cluster-example-1", &clusterMeta) + Expect(err).ToNot(HaveOccurred()) + Expect(modified).To(BeFalse()) Expect(clusterMeta.Annotations). To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-2"))) }) @@ -100,9 +111,11 @@ var _ = Describe("Fencing annotation handling", func() { When("An instance has no annotations", func() { It("should correctly fence it", func() { clusterMeta := metav1.ObjectMeta{} - err := AddFencedInstance("cluster-example-1", &clusterMeta) + modified, err := AddFencedInstance("cluster-example-1", &clusterMeta) Expect(err).NotTo(HaveOccurred()) - Expect(clusterMeta.Annotations).To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-1"))) + Expect(modified).To(BeTrue()) + Expect(clusterMeta.Annotations). + To(HaveKeyWithValue(FencedInstanceAnnotation, jsonMarshal("cluster-example-1"))) }) }) }) diff --git a/tests/utils/fence.go b/tests/utils/fence.go index 39af0df6c9..bbbc52491d 100644 --- a/tests/utils/fence.go +++ b/tests/utils/fence.go @@ -19,7 +19,9 @@ package utils import ( "fmt" - "github.com/cloudnative-pg/cloudnative-pg/pkg/resources" + "k8s.io/apimachinery/pkg/types" + + apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/cloudnative-pg/cloudnative-pg/pkg/utils" ) @@ -49,7 +51,10 @@ func FencingOn( return err } case UsingAnnotation: - err := resources.ApplyFenceFunc(env.Ctx, env.Client, clusterName, namespace, serverName, utils.AddFencedInstance) + err := utils.NewFencingMetadataExecutor(env.Client). + AddFencing(). + ForInstance(serverName). + Execute(env.Ctx, types.NamespacedName{Name: clusterName, Namespace: namespace}, &apiv1.Cluster{}) if err != nil { return err } @@ -75,7 +80,10 @@ func FencingOff( return err } case UsingAnnotation: - err := resources.ApplyFenceFunc(env.Ctx, env.Client, clusterName, namespace, serverName, utils.RemoveFencedInstance) + err := utils.NewFencingMetadataExecutor(env.Client). + RemoveFencing(). + ForInstance(serverName). + Execute(env.Ctx, types.NamespacedName{Name: clusterName, Namespace: namespace}, &apiv1.Cluster{}) if err != nil { return err }