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

Flexible subnet configuration #196

Merged
merged 16 commits into from
Jan 19, 2023
Merged

Conversation

AverageMarcus
Copy link
Member

@AverageMarcus AverageMarcus commented Jan 10, 2023

Adds the ability to have more complex subnet layouts and the ability to target which subnets to use for different groups of resources.

Note: This is a breaking change for existing private clusters and will require a change to the provided values. An entry in the changelog and readme have been added with the required details.

Checklist

  • Update changelog in CHANGELOG.md.

@tityosbot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: Marcus Noble <[email protected]>
Signed-off-by: Marcus Noble <[email protected]>
Signed-off-by: Marcus Noble <[email protected]>
Signed-off-by: Marcus Noble <[email protected]>
availabilityZone: c
isPublic: true
tags:
subnet.giantswarm.io/role: bastion
Copy link
Contributor

Choose a reason for hiding this comment

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

having 3 subnets for 1 bastion might be overkill tho :p

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, until an AZ is having issues :p

Copy link
Contributor

@calvix calvix Jan 10, 2023

Choose a reason for hiding this comment

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

still, its a lot of IP wasted, we should put bastions on the smallest possible subnet size which is /28 if I remember correctly

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, the default split still needs some work, there's a LOT of wasted IPs in this but for now it's just an example to get things working.

@calvix
Copy link
Contributor

calvix commented Jan 10, 2023

could you post here some AWSCluster part where the subnets are generated from the config?

Should we also put a comment on that section that it should not by changed unless someone knows exactly what they are doing? I can see customer breaking it

@AverageMarcus
Copy link
Member Author

@calvix
The AWSCluster bit that uses the config is here: https://github.com/giantswarm/cluster-aws/pull/196/files#diff-47a7bf765a02b080d669157b0f0eae0b849b61d65f3ea8de56392a092c108c16R38-R50

Here's an example of an public AWSCluster created using these default values:

apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AWSCluster
metadata:
  annotations:
    aws.cluster.x-k8s.io/external-resource-gc: "true"
    aws.giantswarm.io/dns-mode: public
    aws.giantswarm.io/vpc-mode: public
    meta.helm.sh/release-name: marc2
    meta.helm.sh/release-namespace: org-giantswarm
  creationTimestamp: "2023-01-10T14:24:27Z"
  finalizers:
  - network-topology.finalizers.giantswarm.io
  - awscluster.infrastructure.cluster.x-k8s.io
  - dns-operator-aws.finalizers.giantswarm.io
  - irsa-operator.finalizers.giantswarm.io
  - capa-iam-operator.finalizers.giantswarm.io/nodes
  - capa-iam-operator.finalizers.giantswarm.io/control-plane
  generation: 5
  labels:
    app: cluster-aws
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/version: 0.20.4-ac30c7b44008566b9510b4d1c82ab4e7712ce6a2
    application.giantswarm.io/team: hydra
    cluster.x-k8s.io/cluster-name: marc2
    cluster.x-k8s.io/watch-filter: capi
    giantswarm.io/cluster: marc2
    giantswarm.io/organization: giantswarm
    helm.sh/chart: cluster-aws-0.20.4-ac30c7b44008566b9510b4d1c82ab4e7712ce6a2
    release.giantswarm.io/version: 20.0.0-alpha1
  name: marc2
  namespace: org-giantswarm
spec:
  bastion:
    allowedCIDRBlocks:
    - 0.0.0.0/0
    enabled: false
  controlPlaneEndpoint:
    host: marc2-apiserver-1501704101.eu-west-2.elb.amazonaws.com
    port: 6443
  controlPlaneLoadBalancer:
    crossZoneLoadBalancing: false
    scheme: internet-facing
  identityRef:
    kind: AWSClusterRoleIdentity
    name: default
  network:
    cni:
      cniIngressRules:
      - description: allow AWS CNI traffic across nodes and control plane
        fromPort: -1
        protocol: "-1"
        toPort: -1
    subnets:
    - availabilityZone: eu-west-2a
      cidrBlock: 10.0.0.0/23
      id: subnet-06e2e94b33fcd9105
      isPublic: true
      natGatewayId: nat-0bbd5bdeab09958a9
      routeTableId: rtb-0bad501c2048289fa
      tags:
        Name: marc2-subnet-public-eu-west-2a
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: public
        subnet.giantswarm.io/role: load-balancers
    - availabilityZone: eu-west-2b
      cidrBlock: 10.0.2.0/23
      id: subnet-0afc16969b6d7db05
      isPublic: true
      natGatewayId: nat-096cd482ac5b8783e
      routeTableId: rtb-02a25ba873f39022d
      tags:
        Name: marc2-subnet-public-eu-west-2b
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: public
        subnet.giantswarm.io/role: load-balancers
    - availabilityZone: eu-west-2c
      cidrBlock: 10.0.4.0/23
      id: subnet-0ab82e671944c89f3
      isPublic: true
      natGatewayId: nat-0555f2f4c413b1a5a
      routeTableId: rtb-0a318c343e575a525
      tags:
        Name: marc2-subnet-public-eu-west-2c
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: public
        subnet.giantswarm.io/role: load-balancers
    - availabilityZone: eu-west-2a
      cidrBlock: 10.0.6.0/23
      id: subnet-0b3795750a4c6d21f
      isPublic: false
      routeTableId: rtb-06c619f9cd4506043
      tags:
        Name: marc2-subnet-private-eu-west-2a
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/internal-elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: private
        subnet.giantswarm.io/role: control-plane
    - availabilityZone: eu-west-2b
      cidrBlock: 10.0.8.0/23
      id: subnet-07699ab3a0285bd54
      isPublic: false
      routeTableId: rtb-0c0c90d8d33642c1f
      tags:
        Name: marc2-subnet-private-eu-west-2b
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/internal-elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: private
        subnet.giantswarm.io/role: control-plane
    - availabilityZone: eu-west-2c
      cidrBlock: 10.0.10.0/23
      id: subnet-0b29b7577bd216601
      isPublic: false
      routeTableId: rtb-0e113d272a2b5960b
      tags:
        Name: marc2-subnet-private-eu-west-2c
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/internal-elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: private
        subnet.giantswarm.io/role: control-plane
    - availabilityZone: eu-west-2a
      cidrBlock: 10.0.12.0/24
      id: subnet-0fac143dbee90dfae
      isPublic: true
      natGatewayId: nat-076302a76c3d10e56
      routeTableId: rtb-0dcdc7180e0aa5d9b
      tags:
        Name: marc2-subnet-public-eu-west-2a
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: public
        subnet.giantswarm.io/role: bastion
    - availabilityZone: eu-west-2b
      cidrBlock: 10.0.13.0/24
      id: subnet-0787324651f5717b0
      isPublic: true
      natGatewayId: nat-0a9b6f886203f10b2
      routeTableId: rtb-0bb2a2909ecf3e447
      tags:
        Name: marc2-subnet-public-eu-west-2b
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: public
        subnet.giantswarm.io/role: bastion
    - availabilityZone: eu-west-2c
      cidrBlock: 10.0.14.0/24
      id: subnet-06a3417ad75154374
      isPublic: true
      natGatewayId: nat-01faff055fce38e5a
      routeTableId: rtb-04274a61c946811dc
      tags:
        Name: marc2-subnet-public-eu-west-2c
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: public
        subnet.giantswarm.io/role: bastion
    - availabilityZone: eu-west-2a
      cidrBlock: 10.0.64.0/18
      id: subnet-0f4ecd0f40fdf4df4
      isPublic: false
      routeTableId: rtb-0e194769797fb1acc
      tags:
        Name: marc2-subnet-private-eu-west-2a
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/internal-elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: private
        subnet.giantswarm.io/role: workers
    - availabilityZone: eu-west-2b
      cidrBlock: 10.0.128.0/18
      id: subnet-044b6cbf72c7ebc7e
      isPublic: false
      routeTableId: rtb-0e64b6c58647221d0
      tags:
        Name: marc2-subnet-private-eu-west-2b
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/internal-elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: private
        subnet.giantswarm.io/role: workers
    - availabilityZone: eu-west-2c
      cidrBlock: 10.0.192.0/18
      id: subnet-0711ec447c21a2283
      isPublic: false
      routeTableId: rtb-0b2c8aeada93c0f94
      tags:
        Name: marc2-subnet-private-eu-west-2c
        kubernetes.io/cluster/marc2: shared
        kubernetes.io/role/internal-elb: "1"
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: private
        subnet.giantswarm.io/role: workers
    vpc:
      availabilityZoneSelection: Ordered
      availabilityZoneUsageLimit: 3
      cidrBlock: 10.0.0.0/16
      id: vpc-07de0d4001c625c23
      internetGatewayId: igw-029021f436572ce16
      tags:
        Name: marc2-vpc
        sigs.k8s.io/cluster-api-provider-aws/cluster/marc2: owned
        sigs.k8s.io/cluster-api-provider-aws/role: common
  region: eu-west-2
  sshKeyName: ssh-key

The tags you see are the final set of tags after CAPA has added it's own default tags to the user provided tags.

Signed-off-by: Marcus Noble <[email protected]>
Signed-off-by: Marcus Noble <[email protected]>
Signed-off-by: Marcus Noble <[email protected]>
Signed-off-by: Marcus Noble <[email protected]>
@AverageMarcus AverageMarcus marked this pull request as ready for review January 13, 2023 16:24
@AverageMarcus AverageMarcus requested a review from a team January 13, 2023 16:24
@AverageMarcus
Copy link
Member Author

/test create
/test upgrade

@AverageMarcus
Copy link
Member Author

AverageMarcus commented Jan 13, 2023

ToDo on Monday

  • Test upgrade of private clusters with documented migration steps
  • Test upgrade of private clusters without the migration steps to see how badly things break (and if they are fixable)
    The AWSCluster surfaces the following error:
    no subnets specified, you must specify the subnets when using an umanaged vpc
    The cluster continues to work though but will need intervention before new features can be used / nodes rolled.
  • Update our fork of CAPA to include this PR - Fail creation of machine pool if no subnets matching filters found kubernetes-sigs/cluster-api-provider-aws#3978

@AverageMarcus
Copy link
Member Author

Notes on private cluster upgrades to this version

When upgrading a private cluster, following the migration steps, the upgrade works as expected providing no changes are made to the CIDR blocks.
It is possible to add more CIDR blocks while doing the upgrade providing the VPC has unused IP space but this change wont cause existing resources to be moved into those CIDRs automatically. E.g., you couldn't add a new set of CIDR blocks tagged as control-plane and expect the existing control plane nodes to be replaced as part of this upgrade. (If another change in the upgrade causes the nodes to roll then this will result in the nodes ending up in the correct subnets)
It is not possible to shrink an existing CIDR as part of the upgrade.

@AverageMarcus
Copy link
Member Author

AverageMarcus commented Jan 16, 2023

@AverageMarcus
Copy link
Member Author

/rest create
/test upgrade

@AverageMarcus
Copy link
Member Author

/test create

@AverageMarcus AverageMarcus changed the title WiP: Flexible subnet configuration (Alternative) Flexible subnet configuration Jan 19, 2023
Copy link
Contributor

@calvix calvix left a comment

Choose a reason for hiding this comment

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

changes LGTM, but I feel we need some kind of documentation for this, could we have an example of how to dot the complex subnets in the Readme or some docs/separate-subnets.MC ?

@AverageMarcus AverageMarcus merged commit db03686 into master Jan 19, 2023
@AverageMarcus AverageMarcus deleted the flexible_subnets_alternative branch January 19, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants