Skip to content

[azure] Single-cluster usage guide for AKS TF module #12697

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 14, 2022
Merged

[azure] Single-cluster usage guide for AKS TF module #12697

merged 1 commit into from
Sep 14, 2022

Conversation

nandajavarma
Copy link
Contributor

@nandajavarma nandajavarma commented Sep 6, 2022 •
edited
Loading

Description

This PR adds the first iteration of single cluster terraform module for AKS. We add a new directory install/infra/single-cluster/azure that uses the aks terraform module to create a single-cluster infrastructure for setting up Gitpod.

Related Issue(s)

Fixes #12867

How to test

Please follow the README.md added as a part of this PR.

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@nandajavarma nandajavarma force-pushed the nvn/azure-tf-mod branch 2 times, most recently from 687b0c0 to 0e5258e Compare September 7, 2022 09:09
@nandajavarma nandajavarma changed the base branch from main to nvn/fix-12609 September 7, 2022 09:10
@nandajavarma nandajavarma changed the base branch from nvn/fix-12609 to main September 7, 2022 09:10
@nandajavarma nandajavarma changed the base branch from main to nvn/fix-12609 September 7, 2022 09:11
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2022

⚠️ Hey reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yml was changed and it might be harmful.

@nandajavarma nandajavarma changed the base branch from nvn/fix-12609 to main September 7, 2022 09:13
@nandajavarma nandajavarma marked this pull request as ready for review September 8, 2022 13:54
@nandajavarma nandajavarma requested a review from a team September 8, 2022 13:54
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Sep 8, 2022
nandajavarma
nandajavarma commented Sep 12, 2022
View reviewed changes
install/tests/Makefile
@@ -54,8 +54,7 @@ gcp-kubeconfig:
azure-kubeconfig:
az login --service-principal -u $$ARM_CLIENT_ID -p $$ARM_CLIENT_SECRET --tenant $$ARM_TENANT_ID
export KUBECONFIG=${KUBECONFIG} && \
export resource=$$(echo "$$TF_VAR_TEST_ID" | sed "s/[\\W\\-]//g") && \
az aks get-credentials --name test-cluster-$$resource --resource-group sh-test-$$resource --file ${KUBECONFIG} || echo "No cluster present"
az aks get-credentials --name p$$TF_VAR_TEST_ID-cluster --resource-group p$$TF_VAR_TEST_ID --file ${KUBECONFIG} || echo "No cluster present"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a following PR these names get referred from terraform output

adrienthebo
adrienthebo reviewed Sep 12, 2022
View reviewed changes
Copy link
Contributor

@adrienthebo adrienthebo left a comment •
edited by nandajavarma
Loading

Choose a reason for hiding this comment

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

Added small comments, picked one nit, otherwise LGTM; tomorrow I'll do some functional review on this.

install/infra/single-cluster/azure/README.md
Before starting the installation process, you need:
* An Azure account
- [Create one now by clicking here](https://azure.microsoft.com/en-gb/free/)
- A user account with "Owner" IAM rights on the subscription
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: When I worked on the reference architecture guide I believe I used the Contributor level, can we double check the permission level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I have to be honest, I basically copied this bit from the gitpod-microsoft-aks-guide. I will check this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So regarding this, I am not entirely sure if a fix is needed. Considering the client id and secret are relating to the tenant. Do you mind if we ship this as is for now? We will anyway have to iterate on this, when syncing with the reference architecture.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, let's ship it 😄

@roboquat roboquat merged commit 40b89ce into main Sep 14, 2022
@roboquat roboquat deleted the nvn/azure-tf-mod branch September 14, 2022 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrienthebo adrienthebo adrienthebo approved these changes

Assignees
No one assigned
Labels
release-note-none size/XXL team: delivery Issue belongs to the self-hosted team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[terraform] Setup basic single-cluster setup for Azure terraform module
3 participants
@nandajavarma @adrienthebo @roboquat