Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

quickstarts,examples: expose cluster_region and clarify #231

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iaguis
Copy link
Contributor

@iaguis iaguis commented Mar 30, 2020

The AWS quickstart shows how to configure the AWS_DEFAULT_REGION
environment variable without explaining that this only applies to the
lokoctl S3 backend.

Also, the current examples don't expose the cluster region so a user
might expect AWS_DEFAULT_REGION to change the region where the cluster
is deployed.

This clarifies what AWS_DEFAULT_REGION configures in the quickstart
and exposes a new cluster_region parameter in the example AWS
configurations.

The AWS quickstart shows how to configure the `AWS_DEFAULT_REGION`
environment variable without explaining that this only applies to the
lokoctl S3 backend.

Also, the current examples don't expose the cluster region so a user
might expect `AWS_DEFAULT_REGION` to change the region where the cluster
is deployed.

This clarifies what `AWS_DEFAULT_REGION` configures in the quickstart
and exposes a new `cluster_region` parameter in the example AWS
configurations.
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

Maybe we could make Terraform pick AWS_DEFAULT_REGION for cluster region as well?

@iaguis
Copy link
Contributor Author

iaguis commented Mar 30, 2020

Maybe we could make Terraform pick AWS_DEFAULT_REGION for cluster region as well?

Does having the region be picked up as an env variable make sense? I'd think this should be a cluster configuration (hopefully in git) and not an env variable.

@invidian
Copy link
Member

Does having the region be picked up as an env variable make sense? I'd think this should be a cluster configuration (hopefully in git) and not an env variable.

For me, it seems confusing, that environment variable is only used for S3 storage and not for the cluster. Either we should ask for it explicitly, or pick the default value consistently to avoid "surprising" behavior.

@invidian
Copy link
Member

aws Terraform provider does pick up AWS_DEFAULT_REGION region just fine, it should be very easy to get that working. And if user have a need to override the default for S3 or for the cluster, this can be done using configuration file + variables then.

@@ -70,7 +70,7 @@ or
```console
export AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE
export AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY
export AWS_DEFAULT_REGION=us-east-1
export AWS_DEFAULT_REGION=us-east-1 # This region will be used by the lokoctl S3 backend only
Copy link
Member

Choose a reason for hiding this comment

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

Could we please avoid the future tense here?

"This region is used by..."

@@ -13,6 +13,10 @@ variable "cluster_name" {
default = "lokomotive-cluster"
}

variable "cluster_region" {
Copy link

Choose a reason for hiding this comment

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

this variable is only used for S3 bucket location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This variable configures the region where the cluster is deployed.

The AWS_DEFAULT_REGION env variable is only used for the S3 bucket location in current Lokomotive.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants