-
Notifications
You must be signed in to change notification settings - Fork 190
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
Add ability to run Atlantis as a deployment #315
Conversation
thanks for the contribution @jacob-jameson , we will review shortly |
The lint test failed because I forgot to bump the chart version number. What should I bump the version number to? Does this count as a Major/Minor/Patch update? |
since is not a breaking change let's update the minor version. |
Ignore the previous commit. There is a mistake in the Maintainers section of the Chart |
Could a Maintainer approve the Test Workflows please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just need to dance around other pending approved PRs and bumping chart version
Is there anything I need to do pre-merge? |
You need to set the chart version to |
Is it acceptable to give a reviewer |
no, you should wait until is reviewed and approved and then bump the chart version |
Remove a comment on line 231
I think all linting tests should pass now. Could I please get a code review? |
Can I get a review please? |
we also have the same requirement. LGTM, can we get a review? |
@jacob-jameson could you fix the conflict? Thanks. |
Conflicts have been resolved. Awaiting Review |
@jacob-jameson please update the chart version |
@jacob-jameson I bumped the version for you but now we have a JSON schema validation so please update the values there. |
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
what
My team and I have added the ability to deploy Atlantis using a
deployment
controller rather than astatefulSet
. This also included adding the ability to create a seperate PVC foratlantis-data
, which may help with PR 304 - Move atlantis-data volume to a separate PVC.The type of controller used to deploy Atlantis is determined by a new setting
.Values.atlantisController
which has a default value ofstatefulSet
. In the absence of this setting, astatefulSet
controller will be created by default. This means that the changes are completely backwards compatible with the current defaultvalues.yaml
file.The
deployment
is configured through thevalues.yaml
file in the.Values.deployment
object.In the event that the Atlantis cluster is scaled to zero and quickly scaled to one again, to prevent two pods from simultaneously trying to access the same lock and plan files we use
.Values.deployment.strategy: "Recreate"
by default.why
We use KEDA to scale our Kubernetes clusters based on events, but we would like the ability to scale our Atlantis clusters based on HTTPS events. To do this we need to use the KEDA http-add-on, which currently only supports
deployment
type controllers.tests
Deployment
We tested these changes by deploying Atlantis with
deployment
andstatefulSet
controllers using the modified Helm chart in this request. Both are able to successfully deploy and pass the UI test suggested in the README by runninghelm test my-atlantis
Scaling
When running Atlantis with a
deployment
controller, one of the main concerns is data in the PVC surviving pod failure. To confirm that data in theatlantis-data
directory in the PVC does survive pod failure we;atlantis-data
directoryatlantis-data
directoryBackwards Compatibility
We also used the following command to test the Manifests produced.
helm install atlantis-release atlantis/ --values atlantis/values.yaml --dry-run --debug | tee manifests.txt
I ran this command in three scenarios:
statefulSet
controller)In all of these scenarios the manifests produced were exactly the same. (Tested by comparing stringified YAML output)
references
No references.