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

Use EKS APIs default vpc-cni addon version for ipv6 clusters #5078

Merged
merged 4 commits into from
Apr 7, 2022

Conversation

aclevername
Copy link
Contributor

@aclevername aclevername commented Apr 6, 2022

Description

Closes #5074

We used to set the version of the vpc-cni if no version was set when creating ipv6 clusters. c095e3b this was because the default version available by EKS wasn't supporting ipv6, so we had to mandate a minimum version. Now this minimum version isn't available on the EKS api for 1.22, so we shouldn't default to it, but we should still check the user provided version is greater than it.

green run https://github.com/weaveworks/eksctl-ci/actions/runs/2103683555

@aclevername aclevername requested a review from a team April 6, 2022 14:04
Comment on lines -73 to +79
Name: api.VPCCNIAddon,
Version: "latest",
Name: api.VPCCNIAddon,
},
{
Name: api.KubeProxyAddon,
Version: "latest",
Name: api.KubeProxyAddon,
},
{
Name: api.CoreDNSAddon,
Version: "latest",
Name: api.CoreDNSAddon,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should always check the default version works, encase we have to enforce any versions again in the future

@@ -270,7 +270,6 @@ func (c *ClusterConfig) unsupportedVPCCNIAddonVersion() (bool, error) {
for _, addon := range c.Addons {
if addon.Name == VPCCNIAddon {
if addon.Version == "" {
addon.Version = minimumVPCCNIVersionForIPv6
Copy link
Contributor

Choose a reason for hiding this comment

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

so.... if it's empty we leave it empty?

Copy link
Contributor Author

@aclevername aclevername Apr 6, 2022

Choose a reason for hiding this comment

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

yep. Empty value means the EKS API uses the default value. E.g.:

 eksctl utils describe-addon-versions --name vpc-cni --kubernetes-version 1.22
2022-04-06 15:22:57 [ℹ]  eksctl version 0.88.0
2022-04-06 15:22:57 [ℹ]  using region us-west-2
2022-04-06 15:22:57 [ℹ]  describing addon versions for addon: vpc-cni
{
  Addons: [{
      AddonName: "vpc-cni",
      AddonVersions: [
        {
          AddonVersion: "v1.10.2-eksbuild.1",
          Architecture: ["amd64","arm64"],
          Compatibilities: [{
              ClusterVersion: "1.22",
              DefaultVersion: false,
              PlatformVersions: ["*"]
            }]
        },
        {
          AddonVersion: "v1.10.1-eksbuild.1",
          Architecture: ["amd64","arm64"],
          Compatibilities: [{
              ClusterVersion: "1.22",
              DefaultVersion: true,
              PlatformVersions: ["*"]
            }]
        },
        {
          AddonVersion: "v1.9.3-eksbuild.1",
          Architecture: ["amd64","arm64"],
          Compatibilities: [{
              ClusterVersion: "1.22",
              DefaultVersion: false,
              PlatformVersions: ["*"]
            }]
        },
        {

v1.10.1-eksbuild.1 would be used

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha, thanks

@aclevername aclevername marked this pull request as ready for review April 7, 2022 08:46
@aclevername aclevername enabled auto-merge (squash) April 7, 2022 15:44
@aclevername aclevername disabled auto-merge April 7, 2022 15:44
@aclevername aclevername enabled auto-merge (squash) April 7, 2022 15:44
@aclevername aclevername disabled auto-merge April 7, 2022 15:48
@aclevername aclevername enabled auto-merge (squash) April 7, 2022 15:48
@aclevername aclevername merged commit c90958b into main Apr 7, 2022
@aclevername aclevername deleted the vpc-cni-122 branch April 7, 2022 16:11
SlevinWasAlreadyTaken pushed a commit to SlevinWasAlreadyTaken/eksctl that referenced this pull request Apr 11, 2022
@hspencer77 hspencer77 mentioned this pull request Jul 8, 2022
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Default vpc-cni add-on version not found when creating clusters
3 participants