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

fix(eks): ssm path for gpu optimized ami is invalid #7672

Merged
merged 6 commits into from
May 1, 2020
Merged

fix(eks): ssm path for gpu optimized ami is invalid #7672

merged 6 commits into from
May 1, 2020

Conversation

pahud
Copy link
Contributor

@pahud pahud commented Apr 29, 2020

Commit Message

fix(eks): ssm path for amazon linux 2 gpu ami is invalid (#7672)

According to the document, the path should be /aws/service/eks/optimized-ami/1.15/amazon-linux-2/recommended/image_id

Also fixes #6891

End Commit Message


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 00fc858
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 80b33e2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@pahud
Copy link
Contributor Author

pahud commented Apr 30, 2020

copy @eladb

@eladb eladb changed the title chore(eks): fix parameter store path for amazon linux 2 gpu AMI ID fix(eks): ssm path for amazon linux 2 gpu ami is invalid Apr 30, 2020
eladb
eladb previously requested changes Apr 30, 2020
this.nodeType = props && props.nodeType;
this.kubernetesVersion = props && props.kubernetesVersion || LATEST_KUBERNETES_VERSION;
public constructor(props: EksOptimizedImageProps = { }) {
this.nodeType = props.nodeType ?? NodeType.STANDARD;
Copy link
Contributor

Choose a reason for hiding this comment

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

@pahud I piggy backed on your PR to fix another issue related to EksOptimizedImage (#6891). Can I bug you to add a unit test for this?

Copy link
Contributor Author

@pahud pahud Apr 30, 2020

Choose a reason for hiding this comment

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

Sure! One more thing - as the default cluster creation now goes to 1.15 as the default k8s version. Are we going to change LATEST_KUBERNETES_VERSION to 1.15 or just keep it as is(1.14)? @eladb

// MAINTAINERS: use ./scripts/kube_bump.sh to update LATEST_KUBERNETES_VERSION
const LATEST_KUBERNETES_VERSION = '1.14';

I am afraid changing it to 1.15 will force the work node upgrade though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was running the integ testing.

Now as the default version goes to 1.15, the managed nodegroup AMI goes to 1.15

AMI ID amazon-eks-node-1.15-v20200228 (ami-06abd5347585f6519)

and the self-managed AMI will goes to 1.14 because of the LATEST_KUBERNETES_VERSION

AMI ID amazon-eks-node-1.14-v20200423 (ami-0bc4f8babcbafed56)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should not offer a default Kubernetes version and require that version always be explicitly set. The current behavior is not sustainable

Let’s do it in a separate PR though.

@eladb eladb changed the title fix(eks): ssm path for amazon linux 2 gpu ami is invalid fix(eks): ssm path for gpu optimized ami is invalid Apr 30, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: f616a2e
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot dismissed eladb’s stale review April 30, 2020 12:42

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 219fb3b
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: fc8b8c7
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented May 1, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 8d2b703
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented May 1, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 5861d18 into aws:master May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants