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

use kustomize instead of sed for environment configuration #1785

Merged
merged 3 commits into from
Nov 8, 2023

Conversation

hahn-kev
Copy link
Collaborator

@hahn-kev hahn-kev commented Nov 8, 2023

Fixes #1783

Description

refactored our k8s resources to use kustomize to simplify having different configs per environment

Checklist

  • I have labeled my PR with: bug, feature, engineering, security fix or testing
  • I have performed a self-review of my own code
  • I have reviewed the title & description of this PR which I will use as the squashed PR commit message
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have enabled auto-merge (optional)

to test this I used kubectl --context dallas-rke diff -k . in the staging folder (and similar for prod) to verify that nothing unexpected changes.

@hahn-kev hahn-kev added the engineering Tasks which do not directly relate to a user-facing feature or fix label Nov 8, 2023
@hahn-kev hahn-kev requested a review from rmunn November 8, 2023 05:03
Copy link

github-actions bot commented Nov 8, 2023

Unit Test Results

362 tests   362 ✔️  12s ⏱️
  37 suites      0 💤
    1 files        0

Results for commit 05c4242.

@hahn-kev hahn-kev enabled auto-merge (squash) November 8, 2023 05:06
@rmunn
Copy link
Collaborator

rmunn commented Nov 8, 2023

Still reading through this, but one comment I have is that you have commands like kubectl --context aws-rke in the Makefile. That's a good idea, but we'll need to add something to the README saying "You need to name your kubectl contexts by these names" or else people who have a different name in their .kubectl will have the command fail by default. And I'd also suggest changing the context names to something that includes "languageforge" or "lf" in it, because in my .kubectl I have the following:

- context:
    cluster: aws-rke
    namespace: languagedepot
    user: aws-rke
  name: languagedepot-rke
- context:
    cluster: aws-rke
    namespace: languageforge
    user: aws-rke
  name: languageforge-rke

I should actually rename those to something like language{forge,depot}-prod or {lf,ld}-prod but haven't gotten around to it. The point is that the kubectl context can carry a default namespace along with it, and I use that so I don't see languageforge pods when I'm deploying languagedepot, and vice versa.

Copy link
Collaborator

@rmunn rmunn left a comment

Choose a reason for hiding this comment

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

Looks good to me. I believe the GHA workflows will continue to work with no modifications with this approach, though of course we'll need to see it in action to be 100% certain. But it looks like you have everything covered that I could think of.

@hahn-kev hahn-kev merged commit 4463f0a into develop Nov 8, 2023
17 checks passed
@hahn-kev hahn-kev deleted the chore/use-kustomize branch November 8, 2023 09:16
@hahn-kev
Copy link
Collaborator Author

hahn-kev commented Nov 8, 2023

That's a good point about the context names. I'll update the readme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engineering Tasks which do not directly relate to a user-facing feature or fix
Projects
None yet
2 participants