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

[aws-eks] Cluster Version Pinning #7762

Closed
2 tasks done
pahud opened this issue May 3, 2020 · 4 comments · Fixed by #8889
Closed
2 tasks done

[aws-eks] Cluster Version Pinning #7762

pahud opened this issue May 3, 2020 · 4 comments · Fixed by #8889
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved.

Comments

@pahud
Copy link
Contributor

pahud commented May 3, 2020

The Cluster resource in the eks construct library allows us to create Amaozn EKS cluster without specifying the version, however, when we create self-managed ASG nodegroup we need specify the version before we can construct correct ssm parameter store path for the AMI, which at this moment will honor the hardcoded LATEST_KUBERNETES_VERSION variable. However, if we version bump LATEST_KUBERNETES_VERSION, users with existing cluster and nodegroup will risk upgrading their self-managed nodegroup k8s version. For example 1.14 to 1.15 or even 1.14 to 1.16, while their cluster may remain 1.14.

However, for the Nodegroup resource, we don't have to specify the version as cloudformation will determine the version from the cluster, which ensures the version consistency.

I guess we might need to make the version property mandatory for Cluster and let users determine the version for their cluster as well as self-managed nodegroups if any.

Benefits

  1. cluster version, managed nodegroup version and self-managed nodegroup version would be all consistent
  2. version pinning would be possible
  3. no forced version upgrade for self-managed NG
  4. would be able to construct correct ssm parameter store path based on the user-provided version

Use Case

Proposed Solution

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@pahud pahud added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 3, 2020
@SomayaB SomayaB added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label May 5, 2020
@eladb
Copy link
Contributor

eladb commented May 6, 2020

Yes, this is exactly what I was thinking. We should get make version a required argument.

If we want a nice experience we can offer an extensible enum-like type, which is a common pattern we use:

export class KubernetesVersion {
  public static of(version: string) { return new KubernetesVersion(version); }

  public static V1_14 = KubernetesVersion.of("1.14");
  public static V1_15 = KubernetesVersion.of("1.15");

  private constructor(public readonly version: string) { }
}

Then, usage is:

new Cluster(this, 'MyCluster', {
  version: KubernetesVersion.V1_14,
  // or, if I want to use a version that is not defined in the "enum":
  version: KubernetesVersion.of("1.99")
}

@eladb eladb added the effort/small Small work item – less than a day of effort label May 6, 2020
@pahud
Copy link
Contributor Author

pahud commented May 6, 2020

@eladb Let me make it :-)

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label May 19, 2020
@eladb eladb added this to the EKS Developer Preview milestone Jun 24, 2020
@eladb eladb changed the title EKS cluster version pinning [EKS Feature] cluster version pinning Jun 24, 2020
@eladb eladb removed this from the EKS Developer Preview milestone Jun 24, 2020
@abelmokadem
Copy link
Contributor

Nice to see this one 👍

@eladb eladb changed the title [EKS Feature] cluster version pinning [EKS Feature] Cluster Version Pinning Jun 24, 2020
@pahud
Copy link
Contributor Author

pahud commented Jul 2, 2020

PR WIP. Will create a PR draft today.

@mergify mergify bot closed this as completed in #8889 Jul 3, 2020
mergify bot pushed a commit that referenced this issue Jul 3, 2020
feat(eks): support cluster version pinning

Support cluster version pinning with the mandatory `version` property in the `Cluster` construct.

Fixes:  #7762 

BREAKING CHANGE:
`version` is now a mandatory property


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@iliapolo iliapolo added this to the EKS Dev Preview milestone Aug 10, 2020
@iliapolo iliapolo changed the title [EKS Feature] Cluster Version Pinning [aws-eks] Cluster Version Pinning Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants