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.

Sign up for GitHub

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

added support for system node pools in managedClusters #1475

Closed
wants to merge 11 commits into from
Closed

added support for system node pools in managedClusters #1475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
89b8d83
updated the containerservice sdk
LochanRn Jun 7, 2021
d3ae03e
fixed min one system pool error.
LochanRn Jun 11, 2021
58b57c9
Merge branch 'master' of https://github.com/kubernetes-sigs/cluster-a…
LochanRn Jun 20, 2021
2b7ffa7
Merge branch 'master' of https://github.com/kubernetes-sigs/cluster-a…
LochanRn Jun 20, 2021
f0e74c2
changes for system managed machine pool
LochanRn Jun 25, 2021
34e2e25
Merge branch 'master' of https://github.com/kubernetes-sigs/cluster-a…
LochanRn Jun 25, 2021
25d6f98
minor changes
LochanRn Jun 25, 2021
5342c94
Merge branch 'master' of https://github.com/kubernetes-sigs/cluster-a…
LochanRn Jun 29, 2021
158270a
Merge branch 'master' of https://github.com/kubernetes-sigs/cluster-a…
LochanRn Jun 30, 2021
a488083
Snp sub branch (#9)
LochanRn Jul 6, 2021
be9196b
Merge branch 'kubernetes-sigs:master' into system-node-pool
LochanRn Jul 6, 2021
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
14 changes: 8 additions & 6 deletions azure/services/agentpools/agentpools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package agentpools
import (
"context"

"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice"
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-03-01/containerservice"
Copy link
Contributor

Choose a reason for hiding this comment

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

any chance I can persuade you to upgrade to 2021-05-01 😃 the diff should be basically nothing. if there's any non-trivial diff, you can leave it at 2021-03-01.

Copy link
Member Author

Choose a reason for hiding this comment

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

ack 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

The one minor difference while moving it from 2021-03-01 - 2021-05-01 is

ListClusterAdminCredentials api has an extra param serverFqdn in 2021-05-01. I have kept it to empty string.

"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
"k8s.io/klog/v2"
Expand All @@ -38,6 +38,7 @@ type Spec struct {
Replicas int32
OSDiskSizeGB int32
VnetSubnetID string
Mode string
}

// Reconcile idempotently creates or updates a agent pool, if possible.
Expand All @@ -52,13 +53,14 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {

profile := containerservice.AgentPool{
ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
VMSize: containerservice.VMSizeTypes(agentPoolSpec.SKU),
OsType: containerservice.Linux,
VMSize: &agentPoolSpec.SKU,
OsType: containerservice.OSTypeLinux,
OsDiskSizeGB: &agentPoolSpec.OSDiskSizeGB,
Count: &agentPoolSpec.Replicas,
Type: containerservice.VirtualMachineScaleSets,
Type: containerservice.AgentPoolTypeVirtualMachineScaleSets,
OrchestratorVersion: agentPoolSpec.Version,
VnetSubnetID: &agentPoolSpec.VnetSubnetID,
Mode: containerservice.AgentPoolMode(agentPoolSpec.Mode),
},
}

Expand Down Expand Up @@ -88,10 +90,10 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
existingProfile := containerservice.AgentPool{
ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
VMSize: existingPool.ManagedClusterAgentPoolProfileProperties.VMSize,
OsType: containerservice.Linux,
OsType: containerservice.OSTypeLinux,
OsDiskSizeGB: existingPool.ManagedClusterAgentPoolProfileProperties.OsDiskSizeGB,
Count: existingPool.ManagedClusterAgentPoolProfileProperties.Count,
Type: containerservice.VirtualMachineScaleSets,
Type: containerservice.AgentPoolTypeVirtualMachineScaleSets,
OrchestratorVersion: existingPool.ManagedClusterAgentPoolProfileProperties.OrchestratorVersion,
VnetSubnetID: existingPool.ManagedClusterAgentPoolProfileProperties.VnetSubnetID,
},
Expand Down
8 changes: 4 additions & 4 deletions azure/services/agentpools/agentpools_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"net/http"
"testing"

"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice"
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-03-01/containerservice"
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/to"
Expand Down Expand Up @@ -217,7 +217,7 @@ func TestReconcile(t *testing.T) {
ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
Count: to.Int32Ptr(3),
OsDiskSizeGB: to.Int32Ptr(20),
VMSize: containerservice.VMSizeTypesStandardA1,
VMSize: to.StringPtr(string(containerservice.VMSizeTypesStandardA1)),
OrchestratorVersion: to.StringPtr("9.99.9999"),
ProvisioningState: to.StringPtr("Failed"),
},
Expand All @@ -242,8 +242,8 @@ func TestReconcile(t *testing.T) {
ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{
Count: to.Int32Ptr(2),
OsDiskSizeGB: to.Int32Ptr(100),
VMSize: containerservice.VMSizeTypesStandardD2sV3,
OsType: containerservice.Linux,
VMSize: to.StringPtr(string(containerservice.VMSizeTypesStandardD2sV3)),
OsType: containerservice.OSTypeLinux,
OrchestratorVersion: to.StringPtr("9.99.9999"),
ProvisioningState: to.StringPtr("Succeeded"),
VnetSubnetID: to.StringPtr(""),
Expand Down
2 changes: 1 addition & 1 deletion azure/services/agentpools/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package agentpools
import (
"context"

"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice"
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-03-01/containerservice"
"github.com/Azure/go-autorest/autorest"
"github.com/pkg/errors"

Expand Down
2 changes: 1 addition & 1 deletion azure/services/agentpools/mock_agentpools/agentpools_mock.go
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion azure/services/managedclusters/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package managedclusters
import (
"context"

"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice"
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-03-01/containerservice"
"github.com/Azure/go-autorest/autorest"
"github.com/pkg/errors"

Expand Down
14 changes: 8 additions & 6 deletions azure/services/managedclusters/managedclusters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"fmt"
"net"

"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice"
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-03-01/containerservice"
"github.com/Azure/go-autorest/autorest/to"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
Expand Down Expand Up @@ -124,7 +124,7 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {

managedCluster := containerservice.ManagedCluster{
Identity: &containerservice.ManagedClusterIdentity{
Type: containerservice.SystemAssigned,
Type: containerservice.ResourceIdentityTypeSystemAssigned,
},
Location: &managedClusterSpec.Location,
Tags: *to.StringMapPtr(managedClusterSpec.Tags),
Expand All @@ -146,7 +146,7 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
ClientID: &managedIdentity,
},
AgentPoolProfiles: &[]containerservice.ManagedClusterAgentPoolProfile{},
NetworkProfile: &containerservice.NetworkProfileType{
NetworkProfile: &containerservice.NetworkProfile{
NetworkPlugin: containerservice.NetworkPlugin(managedClusterSpec.NetworkPlugin),
LoadBalancerSku: containerservice.LoadBalancerSku(managedClusterSpec.LoadBalancerSKU),
NetworkPolicy: containerservice.NetworkPolicy(managedClusterSpec.NetworkPolicy),
Expand Down Expand Up @@ -177,14 +177,16 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
}
}

for _, pool := range managedClusterSpec.AgentPools {
for i := range managedClusterSpec.AgentPools {
pool := managedClusterSpec.AgentPools[i]
profile := containerservice.ManagedClusterAgentPoolProfile{
Name: &pool.Name,
VMSize: containerservice.VMSizeTypes(pool.SKU),
VMSize: &pool.SKU,
OsDiskSizeGB: &pool.OSDiskSizeGB,
Count: &pool.Replicas,
Type: containerservice.VirtualMachineScaleSets,
Type: containerservice.AgentPoolTypeVirtualMachineScaleSets,
VnetSubnetID: &managedClusterSpec.VnetSubnetID,
Mode: containerservice.AgentPoolModeSystem,
}
*managedCluster.AgentPoolProfiles = append(*managedCluster.AgentPoolProfiles, profile)
}
Expand Down
2 changes: 1 addition & 1 deletion azure/services/managedclusters/managedclusters_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"net/http"
"testing"

"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice"
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-03-01/containerservice"
"github.com/Azure/go-autorest/autorest"
"github.com/golang/mock/gomock"
. "github.com/onsi/gomega"
Expand Down
2 changes: 1 addition & 1 deletion azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go
View file
Open in desktop

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

20 changes: 0 additions & 20 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,6 @@ spec:
- host
- port
type: object
defaultPoolRef:
description: DefaultPoolRef is the specification for the default pool,
without which an AKS cluster cannot be created.
properties:
name:
description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
TODO: Add other useful fields. apiVersion, kind, uid?'
type: string
type: object
dnsServiceIP:
description: DNSServiceIP is an IP address assigned to the Kubernetes
DNS service. It must be within the Kubernetes service address range
Expand Down Expand Up @@ -147,7 +138,6 @@ spec:
- name
type: object
required:
- defaultPoolRef
- location
- nodeResourceGroupName
- resourceGroupName
Expand Down Expand Up @@ -217,15 +207,6 @@ spec:
- host
- port
type: object
defaultPoolRef:
description: DefaultPoolRef is the specification for the default pool,
without which an AKS cluster cannot be created.
properties:
name:
description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
TODO: Add other useful fields. apiVersion, kind, uid?'
type: string
type: object
dnsServiceIP:
description: DNSServiceIP is an IP address assigned to the Kubernetes
DNS service. It must be within the Kubernetes service address range
Expand Down Expand Up @@ -337,7 +318,6 @@ spec:
- name
type: object
required:
- defaultPoolRef
- location
- resourceGroupName
- sshPublicKey
Expand Down
16 changes: 16 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ spec:
description: AzureManagedMachinePoolSpec defines the desired state of
AzureManagedMachinePool.
properties:
mode:
description: 'Mode - represents mode of an agent pool. Possible values
include: System, User.'
enum:
- System
- User
type: string
osDiskSizeGB:
description: OSDiskSizeGB is the disk size for every machine in this
agent pool. If you specify 0, it will apply the default osDisk size
Expand All @@ -58,6 +65,7 @@ spec:
description: SKU is the size of the VMs in the node pool.
type: string
required:
- mode
- sku
type: object
status:
Expand Down Expand Up @@ -109,6 +117,13 @@ spec:
description: AzureManagedMachinePoolSpec defines the desired state of
AzureManagedMachinePool.
properties:
mode:
description: 'Mode - represents mode of an agent pool. Possible values
include: System, User.'
enum:
- System
- User
type: string
osDiskSizeGB:
description: OSDiskSizeGB is the disk size for every machine in this
agent pool. If you specify 0, it will apply the default osDisk size
Expand All @@ -125,6 +140,7 @@ spec:
description: SKU is the size of the VMs in the node pool.
type: string
required:
- mode
- sku
type: object
status:
Expand Down
40 changes: 40 additions & 0 deletions config/webhook/manifests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,26 @@ webhooks:
resources:
- azuremanagedcontrolplanes
sideEffects: None
- admissionReviewVersions:
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha4-azuremanagedmachinepool
failurePolicy: Fail
name: default.azuremanagedmachinepools.infrastructure.cluster.x-k8s.io
rules:
- apiGroups:
- infrastructure.cluster.x-k8s.io
apiVersions:
- v1alpha4
operations:
- CREATE
- UPDATE
resources:
- azuremanagedmachinepools
sideEffects: None

---
apiVersion: admissionregistration.k8s.io/v1
Expand Down Expand Up @@ -218,3 +238,23 @@ webhooks:
resources:
- azuremanagedcontrolplanes
sideEffects: None
- admissionReviewVersions:
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-infrastructure-cluster-x-k8s-io-v1alpha4-azuremanagedmachinepool
failurePolicy: Fail
name: validation.azuremanagedmachinepools.infrastructure.cluster.x-k8s.io
rules:
- apiGroups:
- infrastructure.cluster.x-k8s.io
apiVersions:
- v1alpha4
operations:
- UPDATE
- DELETE
resources:
- azuremanagedmachinepools
sideEffects: None
4 changes: 0 additions & 4 deletions exp/api/v1alpha3/azuremanagedcontrolplane_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package v1alpha3

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"

Expand Down Expand Up @@ -69,9 +68,6 @@ type AzureManagedControlPlaneSpec struct {
// SSHPublicKey is a string literal containing an ssh public key base64 encoded.
SSHPublicKey string `json:"sshPublicKey"`

// DefaultPoolRef is the specification for the default pool, without which an AKS cluster cannot be created.
DefaultPoolRef corev1.LocalObjectReference `json:"defaultPoolRef"`

// DNSServiceIP is an IP address assigned to the Kubernetes DNS service.
// It must be within the Kubernetes service address range specified in serviceCidr.
// +optional
Expand Down
4 changes: 4 additions & 0 deletions exp/api/v1alpha3/azuremanagedmachinepool_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import (

// AzureManagedMachinePoolSpec defines the desired state of AzureManagedMachinePool.
type AzureManagedMachinePoolSpec struct {
// Mode - represents mode of an agent pool. Possible values include: System, User.
// +kubebuilder:validation:Enum=System;User
Mode string `json:"mode"`
Copy link
Contributor

Choose a reason for hiding this comment

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

should we default this? or require it? i'm fine as-is but thought I'd raise it

Copy link
Member Author

Choose a reason for hiding this comment

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

I am okay with anything

Copy link
Member Author

Choose a reason for hiding this comment

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

Have we decided on this ? Should I add a default instead of required ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Changing from required => optional is non-breaking, let’s leave it required for now 👍


// SKU is the size of the VMs in the node pool.
SKU string `json:"sku"`

Expand Down
4 changes: 2 additions & 2 deletions exp/api/v1alpha3/zz_generated.conversion.go
View file
Open in desktop

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

1 change: 0 additions & 1 deletion exp/api/v1alpha3/zz_generated.deepcopy.go
View file
Open in desktop

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

3 changes: 0 additions & 3 deletions exp/api/v1alpha4/azuremanagedcontrolplane_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ type AzureManagedControlPlaneSpec struct {
// SSHPublicKey is a string literal containing an ssh public key base64 encoded.
SSHPublicKey string `json:"sshPublicKey"`

// DefaultPoolRef is the specification for the default pool, without which an AKS cluster cannot be created.
DefaultPoolRef corev1.LocalObjectReference `json:"defaultPoolRef"`

// DNSServiceIP is an IP address assigned to the Kubernetes DNS service.
// It must be within the Kubernetes service address range specified in serviceCidr.
// +optional
Expand Down
Loading