Skip to content

feat: Added support for persisting Atlantis state using EFS #247

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

Merged
merged 26 commits into from
Feb 10, 2022

Conversation

MarkIannucci
Copy link
Contributor

Description

Use EFS to store Atlantis state (we are most concerned about locks, but this also reduces repo checkouts in some cases).

Motivation and Context

Fixes #206

Breaking Changes

Reduces the flexibility we offer by requiring the user variable to be in the uid:gid format. This is because I wanted to use the user variable for the posix_user defined in the aws_efs_access_point and I wasn't aware of a method to decompose the more flexible entries we allowed previously into their uid and gid parts. If someone is aware of a more elegant way to do that, I look forward to learning from your contribution! In the meantime, I added a validation to the variable to surface the breaking change at plantime rather than after the deployment.

I also change the default maximum deployment percent and minimum healthy deployment percent to 100 and 0 respectively. I made this change because I found that Atlantis gets angry when there are two instances sharing the same BoltDB database. This means that we have brief interruptions when we make changes that require a new ECS deployment in AWS, but I think it is better to have the interruptions than lost locks.

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects --- I tested this with the github example.

@MarkIannucci MarkIannucci changed the title Persist Atlantis state using EFS fix: Persist Atlantis state using EFS Jan 30, 2022
Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Few comments mostly about code style to match the rest of the module(s) we have.

@MarkIannucci
Copy link
Contributor Author

@antonbabenko , Thanks for the thorough review. We differed in one place on line 205 -- I left the comment unresolved to highlight it. This is ready for a re-review and potential completion.

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Just a comment about default values. Also, please make CI green by running pre-commit run -a and committing the changes.

@antonbabenko antonbabenko changed the title fix: Persist Atlantis state using EFS feat: Added support for persisting Atlantis state using EFS Feb 10, 2022
@antonbabenko antonbabenko merged commit 79d152e into terraform-aws-modules:master Feb 10, 2022
@antonbabenko
Copy link
Member

Thanks @MarkIannucci for the contribution!

antonbabenko pushed a commit that referenced this pull request Feb 10, 2022
## [3.10.0](v3.9.0...v3.10.0) (2022-02-10)

### Features

* Added support for persisting Atlantis state using EFS ([#247](#247)) ([79d152e](79d152e))
@antonbabenko
Copy link
Member

This PR is included in version 3.10.0 🎉

# we coalescelist in order to specify the resource keys when we create the subnets using the VPC or they're specified for us. This works around the for_each value depends on attributes which can't be determined until apply error
for_each = zipmap(coalescelist(var.private_subnets, var.private_subnet_ids), local.private_subnet_ids)

file_system_id = aws_efs_file_system.this[0].id

Choose a reason for hiding this comment

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

@MarkIannucci
first of all, thank you! 🙏

Unfortunately this is not working if I want to have enable_ephemeral_storage=true

╷
│ Error: Invalid index
│ 
│   on .terraform/modules/atlantis/main.tf line 438, in resource "aws_efs_mount_target" "this":
│  438:   file_system_id  = aws_efs_file_system.this[0].id
│     ├────────────────
│     │ aws_efs_file_system.this is empty tuple
│ 
│ The given key does not identify an element in this collection value.
╵

there is no condition for var.enable_ephemeral_storage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@borissavelev , thank you for the bug report. I'm sorry about the problem. I created #254 to track the resolution.

Choose a reason for hiding this comment

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

no problem! thank you)

vpc_id = local.vpc_id
description = "Security group allowing access to the EFS storage"

ingress_cidr_blocks = [var.cidr]

Choose a reason for hiding this comment

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

var.cidr can be empty when is reusing existing VPC

Choose a reason for hiding this comment

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

probably ingress_cidr_blocks is unnecessary here because later we have ingress_with_source_security_group_id

titanlien pushed a commit to Flaconi/terraform-aws-atlantis that referenced this pull request Jun 8, 2022
titanlien pushed a commit to Flaconi/terraform-aws-atlantis that referenced this pull request Jun 8, 2022
titanlien pushed a commit to Flaconi/terraform-aws-atlantis that referenced this pull request Jun 8, 2022
titanlien pushed a commit to Flaconi/terraform-aws-atlantis that referenced this pull request Jun 8, 2022
titanlien pushed a commit to Flaconi/terraform-aws-atlantis that referenced this pull request Jun 9, 2022
titanlien pushed a commit to Flaconi/terraform-aws-atlantis that referenced this pull request Jun 9, 2022
@github-actions
Copy link

github-actions bot commented Nov 8, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2022
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.

Leverage EFS to persist atlantis locks between deployments?
3 participants