Skip to content

Commit

Permalink
Fix idempotent restore when setting ControlPlaneEndpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
mdbooth committed Apr 11, 2024
1 parent 626738a commit 4ce032d
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 6 deletions.
12 changes: 12 additions & 0 deletions api/v1alpha6/openstackcluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ var v1alpha6OpenStackClusterRestorer = conversion.RestorerFor[*OpenStackCluster]
return &c.Spec
},
restorev1alpha6ClusterSpec,
// Filter out ControlPlaneEndpoint, which is written by the
// cluster controller
conversion.HashedFilterField[*OpenStackCluster](
func(s *OpenStackClusterSpec) *OpenStackClusterSpec {
if s.ControlPlaneEndpoint != (clusterv1.APIEndpoint{}) {
f := *s
f.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
return &f
}
return s
},
),
),
"status": conversion.HashedFieldRestorer(
func(c *OpenStackCluster) *OpenStackClusterStatus {
Expand Down
8 changes: 5 additions & 3 deletions api/v1alpha7/openstackcluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@ var v1alpha7OpenStackClusterRestorer = conversion.RestorerFor[*OpenStackCluster]
return &c.Spec
},
restorev1alpha7ClusterSpec,
// Filter out Bastion, which is restored separately
conversion.HashedFilterField[*OpenStackCluster, OpenStackClusterSpec](
// Filter out Bastion, which is restored separately, and
// ControlPlaneEndpoint, which is written by the cluster controller
conversion.HashedFilterField[*OpenStackCluster](
func(s *OpenStackClusterSpec) *OpenStackClusterSpec {
if s.Bastion != nil {
if s.Bastion != nil || s.ControlPlaneEndpoint != (clusterv1.APIEndpoint{}) {
f := *s
f.Bastion = nil
f.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
return &f
}
return s
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/suites/apivalidations/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var _ = Describe("Filter API validations", func() {
// Initialise a basic cluster object in the correct namespace
cluster = &infrav1.OpenStackCluster{}
cluster.Namespace = namespace.Name
cluster.GenerateName = "cluster-"
cluster.GenerateName = clusterNamePrefix
})

DescribeTable("Allow valid neutron filter tags", func(tags []infrav1.FilterByNeutronTags) {
Expand Down
78 changes: 77 additions & 1 deletion test/e2e/suites/apivalidations/openstackcluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1alpha6 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6"
infrav1alpha7 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7"
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
)

Expand Down Expand Up @@ -52,7 +54,7 @@ var _ = Describe("OpenStackCluster API validations", func() {
// Initialise a basic cluster object in the correct namespace
cluster = &infrav1.OpenStackCluster{}
cluster.Namespace = namespace.Name
cluster.GenerateName = "cluster-"
cluster.GenerateName = clusterNamePrefix
})

It("should allow the smallest permissible cluster spec", func() {
Expand Down Expand Up @@ -177,4 +179,78 @@ var _ = Describe("OpenStackCluster API validations", func() {
Expect(create(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
})
})

Context("v1alpha7", func() {
var cluster *infrav1alpha7.OpenStackCluster

BeforeEach(func() {
// Initialise a basic cluster object in the correct namespace
cluster = &infrav1alpha7.OpenStackCluster{}
cluster.Namespace = namespace.Name
cluster.GenerateName = clusterNamePrefix
})

It("should restore cluster spec idempotently after controller writes to controlPlaneEndpoint", func() {
// Set identityRef.Kind, as it will be lost if the restorer does not execute
cluster.Spec.IdentityRef = &infrav1alpha7.OpenStackIdentityReference{
Kind: "FakeKind",
Name: "identity-ref",
}
Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")

// Fetch the infrav1 version of the cluster
infrav1Cluster := &infrav1.OpenStackCluster{}
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, infrav1Cluster)).To(Succeed(), "OpenStackCluster fetch should succeed")

// Update the infrav1 cluster to set the control plane endpoint
infrav1Cluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
Host: "foo",
Port: 1234,
}
Expect(k8sClient.Update(ctx, infrav1Cluster)).To(Succeed(), "Setting control plane endpoint should succeed")

// Fetch the v1alpha7 version of the cluster and ensure that both the new control plane endpoint and the identityRef.Kind are present
cluster = &infrav1alpha7.OpenStackCluster{}
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: infrav1Cluster.Name, Namespace: infrav1Cluster.Namespace}, cluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
Expect(cluster.Spec.ControlPlaneEndpoint).To(Equal(*infrav1Cluster.Spec.ControlPlaneEndpoint), "Control plane endpoint should be restored")
Expect(cluster.Spec.IdentityRef.Kind).To(Equal("FakeKind"), "IdentityRef.Kind should be restored")
})
})

Context("v1alpha6", func() {
var cluster *infrav1alpha6.OpenStackCluster //nolint:staticcheck

BeforeEach(func() {
// Initialise a basic cluster object in the correct namespace
cluster = &infrav1alpha6.OpenStackCluster{} //nolint:staticcheck
cluster.Namespace = namespace.Name
cluster.GenerateName = clusterNamePrefix
})

It("should restore cluster spec idempotently after controller writes to controlPlaneEndpoint", func() {
// Set identityRef.Kind, as it will be lost if the restorer does not execute
cluster.Spec.IdentityRef = &infrav1alpha6.OpenStackIdentityReference{
Kind: "FakeKind",
Name: "identity-ref",
}
Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")

// Fetch the infrav1 version of the cluster
infrav1Cluster := &infrav1.OpenStackCluster{}
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, infrav1Cluster)).To(Succeed(), "OpenStackCluster fetch should succeed")

// Update the infrav1 cluster to set the control plane endpoint
infrav1Cluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
Host: "foo",
Port: 1234,
}
Expect(k8sClient.Update(ctx, infrav1Cluster)).To(Succeed(), "Setting control plane endpoint should succeed")

// Fetch the v1alpha6 version of the cluster and ensure that both the new control plane endpoint and the identityRef.Kind are present
cluster = &infrav1alpha6.OpenStackCluster{} //nolint:staticcheck
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: infrav1Cluster.Name, Namespace: infrav1Cluster.Namespace}, cluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
Expect(cluster.Spec.ControlPlaneEndpoint).To(Equal(*infrav1Cluster.Spec.ControlPlaneEndpoint), "Control plane endpoint should be restored")
Expect(cluster.Spec.IdentityRef.Kind).To(Equal("FakeKind"), "IdentityRef.Kind should be restored")
})
})
})
7 changes: 6 additions & 1 deletion test/e2e/suites/apivalidations/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"path/filepath"
"strconv"
"testing"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -56,6 +57,10 @@ var (
mgrDone chan struct{}
)

const (
clusterNamePrefix = "cluster-"
)

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)

Expand Down Expand Up @@ -144,7 +149,7 @@ var _ = BeforeSuite(func() {
DeferCleanup(func() {
By("Tearing down manager")
mgrCancel()
Eventually(mgrDone).Should(BeClosed(), "Manager should stop")
Eventually(mgrDone).WithTimeout(time.Second*5).Should(BeClosed(), "Manager should stop")
})
})

Expand Down

0 comments on commit 4ce032d

Please sign in to comment.