Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Cecile Robert-Michon committed May 12, 2022
1 parent 5dddbd4 commit e2c6d18
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 77 deletions.
18 changes: 4 additions & 14 deletions azure/scope/managedcontrolplane.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/util/maps"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
clusterv1exp "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/secret"
Expand All @@ -54,11 +53,6 @@ type ManagedControlPlaneScopeParams struct {
ManagedMachinePools []ManagedMachinePool
}

type ManagedMachinePool struct {
InfraMachinePool *infrav1exp.AzureManagedMachinePool
MachinePool *clusterv1exp.MachinePool
}

// NewManagedControlPlaneScope creates a new Scope from the supplied parameters.
// This is meant to be called for each reconcile iteration.
func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlaneScopeParams) (*ManagedControlPlaneScope, error) {
Expand Down Expand Up @@ -421,15 +415,11 @@ func (s *ManagedControlPlaneScope) ManagedClusterSpec(ctx context.Context) azure
}

if clusterNetwork := s.Cluster.Spec.ClusterNetwork; clusterNetwork != nil {
if clusterNetwork.Services != nil {
if len(clusterNetwork.Services.CIDRBlocks) == 1 {
managedClusterSpec.ServiceCIDR = clusterNetwork.Services.CIDRBlocks[0]
}
if clusterNetwork.Services != nil && len(clusterNetwork.Services.CIDRBlocks) == 1 {
managedClusterSpec.ServiceCIDR = clusterNetwork.Services.CIDRBlocks[0]
}
if clusterNetwork.Pods != nil {
if len(clusterNetwork.Pods.CIDRBlocks) == 1 {
managedClusterSpec.PodCIDR = clusterNetwork.Pods.CIDRBlocks[0]
}
if clusterNetwork.Pods != nil && len(clusterNetwork.Pods.CIDRBlocks) == 1 {
managedClusterSpec.PodCIDR = clusterNetwork.Pods.CIDRBlocks[0]
}
}

Expand Down
15 changes: 10 additions & 5 deletions azure/scope/managedmachinepool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
clusterv1exp "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -38,14 +38,18 @@ import (
// ManagedMachinePoolScopeParams defines the input parameters used to create a new managed
// control plane.
type ManagedMachinePoolScopeParams struct {
ManagedMachinePool
Client client.Client
Cluster *clusterv1.Cluster
ControlPlane *infrav1exp.AzureManagedControlPlane
InfraMachinePool *infrav1exp.AzureManagedMachinePool
MachinePool *expv1.MachinePool
ManagedControlPlaneScope azure.ManagedClusterScoper
}

type ManagedMachinePool struct {
InfraMachinePool *infrav1exp.AzureManagedMachinePool
MachinePool *clusterv1exp.MachinePool
}

// NewManagedMachinePoolScope creates a new Scope from the supplied parameters.
// This is meant to be called for each reconcile iteration.
func NewManagedMachinePoolScope(ctx context.Context, params ManagedMachinePoolScopeParams) (*ManagedMachinePoolScope, error) {
Expand Down Expand Up @@ -83,7 +87,7 @@ type ManagedMachinePoolScope struct {

azure.ManagedClusterScoper
Cluster *clusterv1.Cluster
MachinePool *expv1.MachinePool
MachinePool *clusterv1exp.MachinePool
ControlPlane *infrav1exp.AzureManagedControlPlane
InfraMachinePool *infrav1exp.AzureManagedMachinePool
}
Expand Down Expand Up @@ -111,6 +115,7 @@ func (s *ManagedMachinePoolScope) Close(ctx context.Context) error {
return s.PatchObject(ctx)
}

// AgentPoolAnnotations returns a map of annotations for the infra machine pool.
func (s *ManagedMachinePoolScope) AgentPoolAnnotations() map[string]string {
return s.InfraMachinePool.Annotations
}
Expand All @@ -121,7 +126,7 @@ func (s *ManagedMachinePoolScope) AgentPoolSpec() azure.AgentPoolSpec {
}

func buildAgentPoolSpec(managedControlPlane *infrav1exp.AzureManagedControlPlane,
machinePool *expv1.MachinePool,
machinePool *clusterv1exp.MachinePool,
managedMachinePool *infrav1exp.AzureManagedMachinePool) azure.AgentPoolSpec {
var normalizedVersion *string
if machinePool.Spec.Template.Spec.Version != nil {
Expand Down
76 changes: 48 additions & 28 deletions azure/scope/managedmachinepool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ func TestManagedMachinePoolScope_Autoscaling(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
},
},
Expected: azure.AgentPoolSpec{

Expand Down Expand Up @@ -91,8 +93,10 @@ func TestManagedMachinePoolScope_Autoscaling(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithScaling("pool1", 2, 10),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithScaling("pool1", 2, 10),
},
},
Expected: azure.AgentPoolSpec{
Name: "pool1",
Expand Down Expand Up @@ -150,8 +154,10 @@ func TestManagedMachinePoolScope_NodeLabels(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
},
},
Expected: azure.AgentPoolSpec{

Expand Down Expand Up @@ -181,10 +187,12 @@ func TestManagedMachinePoolScope_NodeLabels(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithLabels("pool1", map[string]string{
"custom": "default",
}),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithLabels("pool1", map[string]string{
"custom": "default",
}),
},
},
Expected: azure.AgentPoolSpec{
Name: "pool1",
Expand Down Expand Up @@ -242,8 +250,10 @@ func TestManagedMachinePoolScope_MaxPods(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
},
},
Expected: azure.AgentPoolSpec{
Name: "pool0",
Expand Down Expand Up @@ -272,8 +282,10 @@ func TestManagedMachinePoolScope_MaxPods(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithMaxPods("pool1", 12),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithMaxPods("pool1", 12),
},
},
Expected: azure.AgentPoolSpec{
Name: "pool1",
Expand Down Expand Up @@ -329,8 +341,10 @@ func TestManagedMachinePoolScope_Taints(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
},
},
Expected: azure.AgentPoolSpec{

Expand Down Expand Up @@ -360,14 +374,16 @@ func TestManagedMachinePoolScope_Taints(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithTaints("pool1", infrav1.Taints{
infrav1.Taint{
Key: "key1",
Value: "value1",
Effect: "NoSchedule",
},
}),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithTaints("pool1", infrav1.Taints{
infrav1.Taint{
Key: "key1",
Value: "value1",
Effect: "NoSchedule",
},
}),
},
},
Expected: azure.AgentPoolSpec{
Name: "pool1",
Expand Down Expand Up @@ -423,8 +439,10 @@ func TestManagedMachinePoolScope_OSDiskType(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool0"),
InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem),
},
},
Expected: azure.AgentPoolSpec{
Name: "pool0",
Expand Down Expand Up @@ -453,8 +471,10 @@ func TestManagedMachinePoolScope_OSDiskType(t *testing.T) {
SubscriptionID: "00000000-0000-0000-0000-000000000000",
},
},
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithOsDiskType("pool1", string(containerservice.OSDiskTypeEphemeral)),
ManagedMachinePool: ManagedMachinePool{
MachinePool: getMachinePool("pool1"),
InfraMachinePool: getAzureMachinePoolWithOsDiskType("pool1", string(containerservice.OSDiskTypeEphemeral)),
},
},
Expected: azure.AgentPoolSpec{
Name: "pool1",
Expand Down
11 changes: 5 additions & 6 deletions azure/services/managedclusters/managedclusters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"

"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-05-01/containerservice"
"github.com/Azure/go-autorest/autorest/to"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
Expand Down Expand Up @@ -85,13 +86,11 @@ func (s *Service) Reconcile(ctx context.Context) error {
return errors.Errorf("%T is not a containerservice.ManagedCluster", result)
}
// Update control plane endpoint.
if managedCluster.ManagedClusterProperties != nil && managedCluster.ManagedClusterProperties.Fqdn != nil {
endpoint := clusterv1.APIEndpoint{
Host: *managedCluster.ManagedClusterProperties.Fqdn,
Port: 443,
}
s.Scope.SetControlPlaneEndpoint(endpoint)
endpoint := clusterv1.APIEndpoint{
Host: to.String(managedCluster.ManagedClusterProperties.Fqdn),
Port: 443,
}
s.Scope.SetControlPlaneEndpoint(endpoint)

// Update kubeconfig data
// Always fetch credentials in case of rotation
Expand Down
9 changes: 4 additions & 5 deletions azure/services/managedclusters/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,18 +159,17 @@ type APIServerAccessProfile struct {

var _ azure.ResourceSpecGetterWithHeaders = (*ManagedClusterSpec)(nil)

// ResourceName returns the name of the group.
// ResourceName returns the name of the AKS cluster.
func (s *ManagedClusterSpec) ResourceName() string {
return s.Name
}

// ResourceGroupName returns the name of the group.
// Note that it is the same as the resource name in this case.
// ResourceGroupName returns the name of the resource group.
func (s *ManagedClusterSpec) ResourceGroupName() string {
return s.ResourceGroup
}

// OwnerResourceName is a no-op for groups.
// OwnerResourceName is a no-op for managed clusters.
func (s *ManagedClusterSpec) OwnerResourceName() string {
return "" // not applicable
}
Expand All @@ -180,7 +179,7 @@ func (s *ManagedClusterSpec) CustomHeaders() map[string]string {
return s.Headers
}

// Parameters returns the parameters for the group.
// Parameters returns the parameters for the managed clusters.
func (s *ManagedClusterSpec) Parameters(existing interface{}) (params interface{}, err error) {
decodedSSHPublicKey, err := base64.StdEncoding.DecodeString(s.SSHPublicKey)
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion azure/services/managedclusters/spec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ func TestParameters(t *testing.T) {
g.Expect(result.(containerservice.ManagedCluster).KubernetesVersion).To(Equal(to.StringPtr("v1.22.99")))
},
},
// TODO (cecile): add tests for specific parameters
}
for _, tc := range testcases {
tc := tc
Expand Down
2 changes: 0 additions & 2 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"k8s.io/apimachinery/pkg/runtime"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
Expand All @@ -50,7 +49,6 @@ func (m *AzureManagedControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) err

// Default implements webhook.Defaulter so a webhook will be registered for the type.
func (m *AzureManagedControlPlane) Default(_ client.Client) {
klog.Info("defaulting AzureManagedControlPlane")
if m.Spec.NetworkPlugin == nil {
networkPlugin := "azure"
m.Spec.NetworkPlugin = &networkPlugin
Expand Down
17 changes: 6 additions & 11 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func TestDefaultingWebhook(t *testing.T) {
Expand All @@ -40,8 +39,7 @@ func TestDefaultingWebhook(t *testing.T) {
Version: "1.17.5",
},
}
var client client.Client
amcp.Default(client)
amcp.Default(nil)
g.Expect(*amcp.Spec.NetworkPlugin).To(Equal("azure"))
g.Expect(*amcp.Spec.LoadBalancerSKU).To(Equal("Standard"))
g.Expect(*amcp.Spec.NetworkPolicy).To(Equal("calico"))
Expand All @@ -66,7 +64,7 @@ func TestDefaultingWebhook(t *testing.T) {
amcp.Spec.VirtualNetwork.Subnet.Name = "fooSubnetName"
amcp.Spec.SKU.Tier = PaidManagedControlPlaneTier

amcp.Default(client)
amcp.Default(nil)
g.Expect(*amcp.Spec.NetworkPlugin).To(Equal(netPlug))
g.Expect(*amcp.Spec.LoadBalancerSKU).To(Equal(lbSKU))
g.Expect(*amcp.Spec.NetworkPolicy).To(Equal(netPol))
Expand Down Expand Up @@ -243,11 +241,10 @@ func TestValidatingWebhook(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
t.Parallel()
var client client.Client
if tt.expectErr {
g.Expect(tt.amcp.ValidateCreate(client)).NotTo(Succeed())
g.Expect(tt.amcp.ValidateCreate(nil)).NotTo(Succeed())
} else {
g.Expect(tt.amcp.ValidateCreate(client)).To(Succeed())
g.Expect(tt.amcp.ValidateCreate(nil)).To(Succeed())
}
})
}
Expand Down Expand Up @@ -306,8 +303,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var client client.Client
err := tc.amcp.ValidateCreate(client)
err := tc.amcp.ValidateCreate(nil)
if tc.wantErr {
g.Expect(err).To(HaveOccurred())
g.Expect(err).To(HaveLen(tc.errorLen))
Expand Down Expand Up @@ -729,8 +725,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) {
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var client client.Client
err := tc.amcp.ValidateUpdate(tc.oldAMCP, client)
err := tc.amcp.ValidateUpdate(tc.oldAMCP, nil)
if tc.wantErr {
g.Expect(err).To(HaveOccurred())
} else {
Expand Down
Loading

0 comments on commit e2c6d18

Please sign in to comment.