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

✨ enable BYO IP for control plane #652

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
1 change: 1 addition & 0 deletions api/v1alpha2/azurecluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint
}

dst.Status.FailureDomains = restored.Status.FailureDomains
dst.Spec.NetworkSpec.PublicIP = restored.Spec.NetworkSpec.PublicIP

for _, restoredSubnet := range restored.Spec.NetworkSpec.Subnets {
if restoredSubnet != nil {
Expand Down
3 changes: 2 additions & 1 deletion api/v1alpha2/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 17 additions & 12 deletions api/v1alpha3/azurecluster_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
"fmt"
"regexp"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
)

Expand All @@ -34,16 +32,8 @@ const (
)

// validateCluster validates a cluster
func (c *AzureCluster) validateCluster() error {
var allErrs field.ErrorList
allErrs = append(allErrs, c.validateClusterSpec()...)
if len(allErrs) == 0 {
return nil
}

return apierrors.NewInvalid(
schema.GroupKind{Group: "infrastructure.cluster.x-k8s.io", Kind: "AzureCluster"},
c.Name, allErrs)
func (c *AzureCluster) validateCluster() field.ErrorList {
return c.validateClusterSpec()
}

// validateClusterSpec validates a ClusterSpec
Expand Down Expand Up @@ -156,6 +146,21 @@ func validateIngressRule(ingressRule *IngressRule, fldPath *field.Path) *field.E
return field.Invalid(fldPath, ingressRule.Priority,
fmt.Sprintf("ingress priorities should be between 100 and 4096"))
}
return nil
}

func validateControlPlaneIP(old, new *PublicIPSpec, fldPath *field.Path) *field.Error {
if old == nil && new != nil {
return field.Invalid(fldPath, new, fmt.Sprintf("setting control plane endpoint after cluster creation is not allowed"))
}
if old != nil && new == nil {
return field.Invalid(fldPath, new, fmt.Sprintf("removing control plane endpoint after cluster creation is not allowed"))
}
if old != nil && new != nil && old.Name != new.Name {
return field.Invalid(fldPath, new, fmt.Sprintf("changing control plane endpoint after cluster creation is not allowed"))
}
if new != nil && new.Name == "" {
return field.Invalid(fldPath, new, fmt.Sprintf("control plane endpoint IP name must be non-empty"))
}
return nil
}
46 changes: 42 additions & 4 deletions api/v1alpha3/azurecluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package v1alpha3

import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
Expand Down Expand Up @@ -48,15 +50,51 @@ func (c *AzureCluster) Default() {
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (c *AzureCluster) ValidateCreate() error {
clusterlog.Info("validate create", "name", c.Name)

return c.validateCluster()
allErrs := c.validateCluster()

// validation is smart/lazy enough to work with the same object compared to itself
// it works well for update to take both objects.
if err := validateControlPlaneIP(
c.Spec.NetworkSpec.PublicIP,
c.Spec.NetworkSpec.PublicIP,
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
field.NewPath("spec").Child("networkSpec").Child("publicIp"),
); err != nil {
allErrs = append(allErrs, err)
}

if len(allErrs) == 0 {
return nil
}

return apierrors.NewInvalid(GroupVersion.WithKind("AzureCluster").GroupKind(), c.Name, allErrs)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (c *AzureCluster) ValidateUpdate(old runtime.Object) error {
func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
clusterlog.Info("validate update", "name", c.Name)

return c.validateCluster()
old := oldRaw.(*AzureCluster)

var allErrs field.ErrorList

// validate cluster may return a list of errors
if errs := c.validateCluster(); errs != nil {
allErrs = append(allErrs, errs...)
}

if err := validateControlPlaneIP(
old.Spec.NetworkSpec.PublicIP,
c.Spec.NetworkSpec.PublicIP,
field.NewPath("spec").Child("networkSpec").Child("publicIp"),
); err != nil {
allErrs = append(allErrs, err)
}

if len(allErrs) == 0 {
return nil
}

return apierrors.NewInvalid(GroupVersion.WithKind("AzureCluster").GroupKind(), c.Name, allErrs)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
Expand Down
16 changes: 15 additions & 1 deletion api/v1alpha3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ type Network struct {

// NetworkSpec specifies what the Azure networking resources should look like.
type NetworkSpec struct {
// PublicIP is the public IP to attach to the Azure load balanacer.
// If users provide this field, we expect name to be set and the IP to be in the cluster resource group.
// We will expect the IP to exist and not create it.
// +optional
PublicIP *PublicIPSpec `json:"publicIp,omitempty"`

// Vnet is the configuration for the Azure virtual network.
// +optional
Vnet VnetSpec `json:"vnet,omitempty"`
Expand Down Expand Up @@ -141,9 +147,17 @@ type IngressRules []*IngressRule
// PublicIP defines an Azure public IP address.
type PublicIP struct {
ID string `json:"id,omitempty"`
Name string `json:"name,omitempty"`
IPAddress string `json:"ipAddress,omitempty"`
DNSName string `json:"dnsName,omitempty"`
Name string `json:"name,omitempty"`
}

// PublicIPSpec defines the inputs to create an Azure public IP address.
type PublicIPSpec struct {
// +kubebuilder:validation:MinLength=1
Name string `json:"name"`
// +kubebuilder:validation:MinLength=1
DNSName string `json:"dnsName"`
}

// LoadBalancer defines an Azure load balancer.
Expand Down
20 changes: 20 additions & 0 deletions api/v1alpha3/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 32 additions & 1 deletion cloud/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package scope
import (
"context"
"fmt"
"hash/fnv"

"github.com/Azure/go-autorest/autorest"
"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand Down Expand Up @@ -104,8 +106,18 @@ func (s *ClusterScope) Network() *infrav1.Network {
return &s.AzureCluster.Status.Network
}

// PublicIPSpec returns the public IP specs.
// PublicIPSpecs returns the public IP specs.
func (s *ClusterScope) PublicIPSpecs() []azure.PublicIPSpec {
// If user provides an API server IP, do not return it return it here.
// We expect it to be created by them and will not reconcile it.
if s.AzureCluster.Spec.NetworkSpec.PublicIP != nil && s.AzureCluster.Spec.NetworkSpec.PublicIP.Name != "" {
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
return []azure.PublicIPSpec{
{
Name: azure.GenerateNodeOutboundIPName(s.ClusterName()),
},
}
}

return []azure.PublicIPSpec{
{
Name: azure.GenerateNodeOutboundIPName(s.ClusterName()),
Expand Down Expand Up @@ -231,3 +243,22 @@ func (s *ClusterScope) SetFailureDomain(id string, spec clusterv1.FailureDomainS
}
s.AzureCluster.Status.FailureDomains[id] = spec
}

// SetAPIServerIP will set the spec for a for a given key
func (s *ClusterScope) SetAPIServerIP() error {
// If IP provided by name, expect to exist and don't precreate.
if s.AzureCluster.Spec.NetworkSpec.PublicIP != nil {
s.AzureCluster.Status.Network.APIServerIP.Name = s.AzureCluster.Spec.NetworkSpec.PublicIP.Name
s.AzureCluster.Status.Network.APIServerIP.DNSName = s.AzureCluster.Spec.NetworkSpec.PublicIP.DNSName
} else if s.Network().APIServerIP.Name == "" {
// otherwise, generate
h := fnv.New32a()
if _, err := h.Write([]byte(fmt.Sprintf("%s/%s/%s", s.SubscriptionID(), s.ResourceGroup(), s.ClusterName()))); err != nil {
return errors.Wrapf(err, "failed to write hash sum for api server ip")
}
s.AzureCluster.Status.Network.APIServerIP.Name = azure.GeneratePublicIPName(s.ClusterName(), fmt.Sprintf("%x", h.Sum32()))
s.AzureCluster.Status.Network.APIServerIP.DNSName = s.GenerateFQDN()
}

return nil
}
13 changes: 7 additions & 6 deletions cloud/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,14 @@ type MachineScope struct {

// PublicIPSpec returns the public IP specs.
func (m *MachineScope) PublicIPSpecs() []azure.PublicIPSpec {
var spec []azure.PublicIPSpec
if m.AzureMachine.Spec.AllocatePublicIP == true {
spec = append(spec, azure.PublicIPSpec{
Name: azure.GenerateNodePublicIPName(m.Name()),
})
if m.AzureMachine.Spec.AllocatePublicIP {
return []azure.PublicIPSpec{
{
Name: azure.GenerateNodePublicIPName(azure.GenerateNICName(m.Name())),
},
}
}
return spec
return nil
}

// NICSpecs returns the network interface specs.
Expand Down
63 changes: 34 additions & 29 deletions cloud/services/publicips/publicips.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,51 +28,56 @@ import (

// Reconcile gets/creates/updates a public ip.
func (s *Service) Reconcile(ctx context.Context) error {
for _, ip := range s.Scope.PublicIPSpecs() {
s.Scope.V(2).Info("creating public IP", "public ip", ip.Name)
err := s.Client.CreateOrUpdate(
ctx,
s.Scope.ResourceGroup(),
ip.Name,
network.PublicIPAddress{
Sku: &network.PublicIPAddressSku{Name: network.PublicIPAddressSkuNameStandard},
Name: to.StringPtr(ip.Name),
Location: to.StringPtr(s.Scope.Location()),
PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
PublicIPAddressVersion: network.IPv4,
PublicIPAllocationMethod: network.Static,
DNSSettings: &network.PublicIPAddressDNSSettings{
DomainNameLabel: to.StringPtr(strings.ToLower(ip.Name)),
Fqdn: to.StringPtr(ip.DNSName),
},
for _, publicIPSpec := range s.Scope.PublicIPSpecs() {
s.Scope.V(2).Info("creating public IP", "public-ip", publicIPSpec.Name)

ipSpec := network.PublicIPAddress{
Sku: &network.PublicIPAddressSku{Name: network.PublicIPAddressSkuNameStandard},
Name: to.StringPtr(publicIPSpec.Name),
Location: to.StringPtr(s.Scope.Location()),
PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
PublicIPAddressVersion: network.IPv4,
PublicIPAllocationMethod: network.Static,
DNSSettings: &network.PublicIPAddressDNSSettings{
DomainNameLabel: to.StringPtr(strings.ToLower(publicIPSpec.Name)),
Fqdn: to.StringPtr(publicIPSpec.DNSName),
},
},
)
}

if err != nil {
if err := s.Client.CreateOrUpdate(
ctx,
s.Scope.ResourceGroup(),
publicIPSpec.Name,
ipSpec,
); err != nil {
return errors.Wrap(err, "cannot create public IP")
}

s.Scope.V(2).Info("successfully created public IP", "public ip", ip.Name)
s.Scope.V(2).Info("successfully created public IP", "public ip", publicIPSpec.Name)
}

return nil
}

// Delete deletes the public IP with the provided scope.
func (s *Service) Delete(ctx context.Context) error {
for _, ip := range s.Scope.PublicIPSpecs() {
s.Scope.V(2).Info("deleting public IP", "public ip", ip.Name)
err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), ip.Name)
if err != nil && azure.ResourceNotFound(err) {
// already deleted
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch 😬

for _, publicIPSpec := range s.Scope.PublicIPSpecs() {
s.Scope.V(2).Info("deleting public IP", "public ip", publicIPSpec.Name)

err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), publicIPSpec.Name)
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
s.Scope.V(2).Info("deleted public IP", "public ip", publicIPSpec.Name)
continue
}
if err != nil {
return errors.Wrapf(err, "failed to delete public IP %s in resource group %s", ip.Name, s.Scope.ResourceGroup())

if azure.ResourceNotFound(err) {
s.Scope.V(2).Info("tried to delete public IP which didn't exist", "public ip", publicIPSpec.Name)
continue
}

s.Scope.V(2).Info("deleted public IP", "public ip", ip.Name)
return errors.Wrapf(err, "failed to delete public IP %s in resource group %s", publicIPSpec.Name, s.Scope.ResourceGroup())
}

return nil
}
Loading