Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Support externally managed cluster infrastructure #307

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/v1beta3/cloudstackcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/cluster-api-provider-cloudstack/pkg/webhookutil"
"sigs.k8s.io/cluster-api/util/annotations"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
Expand Down Expand Up @@ -109,6 +110,13 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
"controlplaneendpoint.port", errorList)
}

if annotations.IsExternallyManaged(oldCluster) && !annotations.IsExternallyManaged(r) {
errorList = append(errorList,
field.Forbidden(field.NewPath("metadata", "annotations"),
"removal of externally managed (managed-by) annotation is not allowed"),
)
}

return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
}

Expand Down
22 changes: 22 additions & 0 deletions api/v1beta3/cloudstackcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
. "github.com/onsi/gomega"
infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3"
dummies "sigs.k8s.io/cluster-api-provider-cloudstack/test/dummies/v1beta3"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/annotations"
)

var _ = Describe("CloudStackCluster webhooks", func() {
Expand Down Expand Up @@ -81,4 +83,24 @@ var _ = Describe("CloudStackCluster webhooks", func() {
Should(MatchError(MatchRegexp(forbiddenRegex, "controlplaneendpoint\\.port")))
})
})

Context("When updating a CloudStackCluster's annotations", func() {
It("Should reject removal of externally managed ('managed-by') annotation from CloudStackCluster", func() {
// Create a CloudStackCluster with managed-by annotation
annotations.AddAnnotations(dummies.CSCluster, map[string]string{clusterv1.ManagedByAnnotation: ""})
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed())

// Remove the annotation and update CloudStackCluster
dummies.CSCluster.Annotations = make(map[string]string)
Ω(k8sClient.Update(ctx, dummies.CSCluster)).
Should(MatchError(MatchRegexp(forbiddenRegex, "removal of externally managed")))
})

It("Should allow adding of externally managed ('managed-by') annotation to CloudStackCluster", func() {
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed())

annotations.AddAnnotations(dummies.CSCluster, map[string]string{clusterv1.ManagedByAnnotation: ""})
Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(Succeed())
})
})
})
28 changes: 25 additions & 3 deletions controllers/cloudstackcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,21 @@ import (
"reflect"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/pkg/errors"
infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3"
csCtrlrUtils "sigs.k8s.io/cluster-api-provider-cloudstack/controllers/utils"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/predicates"
)

Expand Down Expand Up @@ -183,16 +186,35 @@ func (reconciler *CloudStackClusterReconciler) SetupWithManager(ctx context.Cont
return !reflect.DeepEqual(oldCluster, newCluster)
},
},
).Build(reconciler)
).
WithEventFilter(predicates.ResourceIsNotExternallyManaged(log)).
Build(reconciler)
if err != nil {
return errors.Wrap(err, "building CloudStackCluster controller")
}

// Add a watch on CAPI Cluster objects for unpause and ready events.
clusterToInfraFn := util.ClusterToInfrastructureMapFunc(ctx, infrav1.GroupVersion.WithKind("CloudStackCluster"), mgr.GetClient(), &infrav1.CloudStackCluster{})
if err = controller.Watch(
&source.Kind{Type: &clusterv1.Cluster{}},
handler.EnqueueRequestsFromMapFunc(
util.ClusterToInfrastructureMapFunc(ctx, infrav1.GroupVersion.WithKind("CloudStackCluster"), mgr.GetClient(), &infrav1.CloudStackCluster{})),
handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request {
requests := clusterToInfraFn(o)
if requests == nil {
return nil
}

c := &infrav1.CloudStackCluster{}
if err := reconciler.K8sClient.Get(ctx, requests[0].NamespacedName, c); err != nil {
log.V(4).Error(err, "Failed to get CloudStack cluster")
return nil
}

if annotations.IsExternallyManaged(c) {
log.V(4).Info("CloudStackCluster is externally managed, skipping mapping")
return nil
}
return requests
}),
predicates.ClusterUnpaused(log),
); err != nil {
return errors.Wrap(err, "building CloudStackCluster controller")
Expand Down
3 changes: 2 additions & 1 deletion controllers/cloudstackmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/predicates"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -126,7 +127,7 @@ func (r *CloudStackMachineReconciliationRunner) Reconcile() (retRes ctrl.Result,
r.ConsiderAffinity,
r.GetOrCreateVMInstance,
r.RequeueIfInstanceNotRunning,
r.AddToLBIfNeeded,
r.RunIf(func() bool { return !annotations.IsExternallyManaged(r.CSCluster) }, r.AddToLBIfNeeded),
r.GetOrCreateMachineStateChecker,
)
}
Expand Down
54 changes: 32 additions & 22 deletions pkg/cloud/isolated_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3"
"sigs.k8s.io/cluster-api/util/annotations"
)

type IsoNetworkIface interface {
Expand Down Expand Up @@ -64,7 +65,12 @@ func (c *client) AssociatePublicIPAddress(
return errors.Wrapf(err, "fetching a public IP address")
}
isoNet.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress
csCluster.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress
if !annotations.IsExternallyManaged(csCluster) {
// Do not update the infracluster's controlPlaneEndpoint when the controlplane
// is externally managed, it is the responsibility of the externally managed
// control plane to update this.
csCluster.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress
}
isoNet.Status.PublicIPID = publicAddress.Id

// Check if the address is already associated with the network.
Expand Down Expand Up @@ -152,11 +158,13 @@ func (c *client) GetPublicIP(
if err != nil {
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
return nil, err
} else if ip != "" && publicAddresses.Count == 1 { // Endpoint specified and IP found.
} else if ip != "" && publicAddresses.Count == 1 {
// Endpoint specified and IP found.
// Ignore already allocated here since the IP was specified.
return publicAddresses.PublicIpAddresses[0], nil
} else if publicAddresses.Count > 0 { // Endpoint not specified.
for _, v := range publicAddresses.PublicIpAddresses { // Pick first available address.
} else if publicAddresses.Count > 0 {
// Endpoint not specified. Pick first available address.
for _, v := range publicAddresses.PublicIpAddresses {
if v.Allocated == "" { // Found un-allocated Public IP.
return v, nil
}
Expand Down Expand Up @@ -220,17 +228,6 @@ func (c *client) GetOrCreateLoadBalancerRule(
isoNet *infrav1.CloudStackIsolatedNetwork,
csCluster *infrav1.CloudStackCluster,
) (retErr error) {
// Check/set ports.
// Prefer control plane endpoint. Take iso net port if CP missing. Set to default if both missing.
if csCluster.Spec.ControlPlaneEndpoint.Port != 0 {
isoNet.Spec.ControlPlaneEndpoint.Port = csCluster.Spec.ControlPlaneEndpoint.Port
} else if isoNet.Spec.ControlPlaneEndpoint.Port != 0 { // Override default public port if endpoint port specified.
csCluster.Spec.ControlPlaneEndpoint.Port = isoNet.Spec.ControlPlaneEndpoint.Port
} else {
csCluster.Spec.ControlPlaneEndpoint.Port = 6443
isoNet.Spec.ControlPlaneEndpoint.Port = 6443
}

// Check if rule exists.
if err := c.ResolveLoadBalancerRuleDetails(fd, isoNet, csCluster); err == nil ||
!strings.Contains(strings.ToLower(err.Error()), "no load balancer rule found") {
Expand Down Expand Up @@ -275,14 +272,27 @@ func (c *client) GetOrCreateIsolatedNetwork(
return errors.Wrapf(err, "tagging network with id %s", networkID)
}

// Associate Public IP with CloudStackIsolatedNetwork
if err := c.AssociatePublicIPAddress(fd, isoNet, csCluster); err != nil {
return errors.Wrapf(err, "associating public IP address to csCluster")
}
if !annotations.IsExternallyManaged(csCluster) {
// Associate Public IP with CloudStackIsolatedNetwork
if err := c.AssociatePublicIPAddress(fd, isoNet, csCluster); err != nil {
return errors.Wrapf(err, "associating public IP address to csCluster")
}

// Check/set ControlPlaneEndpoint port.
// Prefer csCluster ControlPlaneEndpoint port. Use isonet port if CP missing. Set to default if both missing.
if csCluster.Spec.ControlPlaneEndpoint.Port != 0 {
isoNet.Spec.ControlPlaneEndpoint.Port = csCluster.Spec.ControlPlaneEndpoint.Port
} else if isoNet.Spec.ControlPlaneEndpoint.Port != 0 { // Override default public port if endpoint port specified.
csCluster.Spec.ControlPlaneEndpoint.Port = isoNet.Spec.ControlPlaneEndpoint.Port
} else {
csCluster.Spec.ControlPlaneEndpoint.Port = 6443
isoNet.Spec.ControlPlaneEndpoint.Port = 6443
}

// Setup a load balancing rule to map VMs to Public IP.
if err := c.GetOrCreateLoadBalancerRule(fd, isoNet, csCluster); err != nil {
return errors.Wrap(err, "getting or creating load balancing rule")
// Setup a load balancing rule to map VMs to Public IP.
if err := c.GetOrCreateLoadBalancerRule(fd, isoNet, csCluster); err != nil {
return errors.Wrap(err, "getting or creating load balancing rule")
}
}

// Open the Isolated Network on endopint port.
Expand Down
Loading