Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Correcting AMI ID being sent from the UI #558

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

saji-pivotal
Copy link
Contributor

@saji-pivotal saji-pivotal commented Sep 2, 2021

Signed-off-by: Sudarshan asudarshan@vmware.com

What this PR does / why we need it:
This PR fixes the issue where the AWS_AMI_ID was being incorrectly set by the UI

Which issue(s) this PR fixes:

Fixes #713

Describe testing done for PR:

  1. Ran tanzu management-cluster create --ui
  2. Set the OS Image to Ubuntu (OS Image: ubuntu-20.04-amd64 (ami-0c687784b0671fc49)
  3. Check the config file generated has the correct AMI ID
cat /Users/sudarshanaji/.config/tanzu/tkg/clusterconfigs/xxg1qijl5q.yaml | grep AWS_AMI_ID
AWS_AMI_ID: ami-0c687784b0671fc49
  1. Edit the configurations on the UI to select Amazon Linux (OS Image: amazon-2-amd64 (ami-0293f31158f5c72f3))
  2. Check the config file generated has the correct AMI ID
AWS_AMI_ID: ami-0293f31158f5c72f3
  1. Change the OS Image back to Ubuntu, and try to create a management cluster
  2. Management cluster creation succeeded
    Special notes for your reviewer:

Does this PR introduce a user-facing change?:


New PR Checklist

  • Ensure PR contains only public links or terms
  • Use good commit messages
  • Squash the commits in this branch before merge to preserve our git history
  • If this PR is just an idea or POC, use a Draft PR instead of a full PR
  • Add appropriate kind label according to what type of issue is being addressed.

Sorry, something went wrong.

@saji-pivotal saji-pivotal requested a review from a team as a code owner September 2, 2021 18:26
@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/558/20210902183427/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/558/20210914221620/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@vuil vuil added ok-to-merge PRs should be labelled with this before merging kind/bug PR/Issue related to a bug area/ux UX related area/plugin labels Sep 15, 2021
@saji-pivotal saji-pivotal force-pushed the saji/fix-aws-amiid branch 2 times, most recently from d6e59d9 to c281f47 Compare September 28, 2021 22:14
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/558/20210928221508/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/558/20210928222601/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/558/20210928223449/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

pkg/v1/tkg/tkgconfigproviders/aws.go Outdated Show resolved Hide resolved
pkg/v1/tkg/tkgconfigproviders/aws.go Outdated Show resolved Hide resolved
pkg/v1/tkg/tkgconfigproviders/client.go Outdated Show resolved Hide resolved
Signed-off-by: Sudarshan <asudarshan@vmware.com>
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/558/20210929002020/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/558/20210929002104/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Copy link
Contributor

@imikushin imikushin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

LGTM

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area/plugin area/ux UX related kind/bug PR/Issue related to a bug ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The incorrect AMI ID is being passed from the UI to the config file
4 participants