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

examples: aks-production cluster config update #1307

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

Conversation

knrt10
Copy link
Member

@knrt10 knrt10 commented Jan 8, 2021

Add new compatible components which we test on CI. Remove AWS dependant
backend as it should not be needed on AKS cluster.

Signed-off-by: knrt10 kautilya@kinvolk.io

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.

I'm not sure if this PR is really needed.

examples/aks-production/cluster.lokocfg Show resolved Hide resolved
examples/aks-production/cluster.lokocfg Outdated Show resolved Hide resolved
examples/aks-production/cluster.lokocfg Outdated Show resolved Hide resolved
examples/aks-production/cluster.lokocfg Outdated Show resolved Hide resolved
@knrt10
Copy link
Member Author

knrt10 commented Jan 8, 2021

How about adding web-ui?

@invidian
Copy link
Member

invidian commented Jan 8, 2021

How about adding web-ui?

Yeah, I think that make sense 👍

@knrt10 knrt10 force-pushed the knrt10/aks-prod-example-update branch from ccc7c35 to e63a97a Compare January 8, 2021 14:27
@knrt10 knrt10 requested a review from invidian January 8, 2021 14:27
invidian
invidian previously approved these changes Jan 8, 2021
@knrt10
Copy link
Member Author

knrt10 commented Jan 9, 2021

@invidian in production config we are already using backend as s3, so I guess we could add external-dns and ingresses to web-ui?

@invidian
Copy link
Member

invidian commented Jan 9, 2021

@invidian in production config we are already using backend as s3, so I guess we could add external-dns and ingresses to web-ui?

Hmm, true. I guess it's fine to add it then, though still S3 is easier to consume than Route53.

@knrt10 knrt10 force-pushed the knrt10/aks-prod-example-update branch from e63a97a to 2d7716a Compare January 10, 2021 06:54
@knrt10 knrt10 requested a review from invidian January 10, 2021 06:55
invidian
invidian previously approved these changes Jan 10, 2021
@knrt10 knrt10 force-pushed the knrt10/aks-prod-example-update branch from 2d7716a to 21d0151 Compare January 18, 2021 12:09
@knrt10 knrt10 requested a review from invidian January 18, 2021 12:09
@@ -33,21 +31,6 @@ variable "location" {
default = "West Europe"
}

variable "state_s3_key" {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we remove it? For production setups using local state is not recommended. And we do not have an alternative right now. See #361.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I did not know about it, will revert it back

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it is practical for someone to use S3 when they are deploying cluster to AKS.

It is more practical to store the assets locally than someone creating an account on AWS just for storing assets on S3. I think we should not add S3 or any AWS related stuff for Azure. It is not how production setup for a pragmatic user will look like.

Copy link
Member

Choose a reason for hiding this comment

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

Either we add a way to store assets on Azure and then add that to this docs OR use local dir to store assets.

Copy link
Member

Choose a reason for hiding this comment

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

Why do you say it's not production ready? I don't think practicality plays a role in reliability/robustness required to treat something as "production-ready".

Copy link
Member

Choose a reason for hiding this comment

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

The goal of this document is to be useful not just something that works. Having two cloud accounts to run a cluster on one is not something one would do in production, unless one cloud lacks the functionality. In this case Azure has a functionality we need to implement it or suggest using local dir for assets.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should remove production config for AKS then until we implement Azure backend support.

Copy link
Member

@surajssd surajssd Jan 29, 2021

Choose a reason for hiding this comment

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

Or And leave this PR as it is for now.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "leave"? To keep it open?

I think we should remove the production configuration.

Add new compatible components which we test on CI.

Signed-off-by: knrt10 <kautilya@kinvolk.io>
@knrt10 knrt10 force-pushed the knrt10/aks-prod-example-update branch from 21d0151 to 52aaf44 Compare January 18, 2021 12:41
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.

3 participants