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/ running ArgoCd example on GovCloud #397

Merged
merged 5 commits into from
Apr 21, 2022

Conversation

camba1
Copy link
Contributor

@camba1 camba1 commented Apr 10, 2022

What does this PR do?

PR has two changes to enhance the experience of running the argocd example in govCloud:

  • Remove hardcoded region and move it into a variable that can more easily overridden
  • Update the Nginx module to create the nginx namespace instead of the ingress-nginx namespace. With the old namespace nothing would get created for Nginx beyond an empty namespace and the following error could be seen on Argo cd:
    Failed sync attempt to c0d70e7b110803dd967c91fec4cba9bd9622bcab: one or more objects failed to apply, reason: namespaces "nginx" not found (retried 1 times).
    Once the correct namespace is created, everything syncs properly and all appropriate resources are created

Motivation

Testing the ArgoCd example in govcloud to see if gitops would work as expected.

Additional Notes

For the ArgoCD example to be fully functional, it requires that PR 381 is passed as well to work properly, without that PR the load balancer will fail to deploy.

Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

Thanks @camba1 👍🏼 LGTM

@@ -34,7 +34,7 @@ data "aws_eks_cluster_auth" "cluster" {
}

provider "aws" {
region = "us-west-2"
region = var.region
Copy link
Contributor

Choose a reason for hiding this comment

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

Like that. Should be done for all examples.

Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

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

@camba1 thanks for the PR. Question on the changing namespace.

modules/kubernetes-addons/ingress-nginx/locals.tf Outdated Show resolved Hide resolved
@camba1 camba1 requested a review from askulkarni2 April 14, 2022 17:18
Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@askulkarni2 askulkarni2 merged commit 6d50f39 into aws-ia:main Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants