Fix Anubis Setup Destroy #768
Conversation
Chancebair
commented
Aug 22, 2019
Chancebair
commented
- Adds a sync-infra target
- Fixes pipeline destroy by manually revoking security group association
- Undeploys the bai-bff loadbalancer
| ): | ||
| group_id = subprocess.check_output(["terraform", "output", "blackbox_vpc_default_group_id"]).strip() | ||
| source_group = subprocess.check_output(["terraform", "output", "blackbox_public_group_id"]).strip() | ||
| return_code = subprocess.call( |
There was a problem hiding this comment.
Could you elaborate why this is necessary? We should not use the aws cli in any case because any resource that exists in this account, was created by some kind of stateful automation. The removal should then happen through the same automation, as otherwise we're corrupting the state.
There was a problem hiding this comment.
Terraform isn't smart enough to disassociate dependent rules before deleting security groups, therefore it will just hang on "deleting security group" when the console prompts that there are dependent references of that security group elsewhere
There was a problem hiding this comment.
Who created and associated these security groups?
There was a problem hiding this comment.
@marcoabreu I get where you are coming from... we should walk the same path forward and we do backward. This shows that an out-of-band actor is inserted in the reverse path. It turns out that this is somewhat avoidable as there is not this affordance directly in TF.
So this is unfortunately the way we need to go. @Chancebair Please document this caveat so that posterity is aware of this decision point.
There was a problem hiding this comment.
Commented the hack in the script
There was a problem hiding this comment.
Which resource specifically has these groups assigned so that we run into that bug? Is it an instance, an ELB or something else? Also, it would be great if you could point me to the line where this "buggy" reference is being created in the first place.
| destroy_pipeline(region) | ||
| undeploy_services() |
There was a problem hiding this comment.
Is this the right order of execution? I'd expect undeploy services to happen first
There was a problem hiding this comment.
The pipeline and the services are mutually exclusive, it doesn't matter
| print(f"=> Calling `./baictl sync infra --aws-region={region}` in baictl to get kubeconfig") | ||
|
|
||
| if os.path.exists(os.path.join(os.path.dirname(__file__), "../baictl/drivers/aws/cluster/.terraform")): | ||
| return_code = subprocess.call(["rm", "-rf", "drivers/aws/cluster/.terraform"], cwd="../baictl") |
There was a problem hiding this comment.
os.path.join(os.path.dirname(file)
There was a problem hiding this comment.
You still have relative references based on the users CWD in your code.
| ] | ||
| ) | ||
| devnull = open(os.devnull, "w") | ||
| if ( |
There was a problem hiding this comment.
I'm still not convinced that this is the right thing to do. I rather feel like we're working around an issue here, so I'd like to see this elaborated.
There was a problem hiding this comment.
There was a problem hiding this comment.
@marcoabreu
Your objection is reasonable and noted - Now, could you, please offer an alternative solution that can be implemented within a reasonable time.
There was a problem hiding this comment.
I'm still trying to understand which resource specifically has this security group assigned. So far, the situation is not very transparent. As soon as I have all the data, I'm happy to assist with a solution. But so far, I'm still at the information-gathering stage.
|
|
||
|
|
||
| def undeploy_services(): | ||
| parent_dir = os.path.join(os.path.dirname(__file__), "..") |
There was a problem hiding this comment.
What about guys, who are not deployable?
puller/kafka-utils/metrics-pusher?
Do they have stubs, right?
There was a problem hiding this comment.
Currently its broken on #688 so I can't get any further to test
| service_dirs = [ | ||
| f | ||
| for f in os.listdir(parent_dir) | ||
| if os.path.isdir(os.path.join(parent_dir, f)) and os.path.exists(os.path.join(parent_dir, f) + "/Makefile") |
There was a problem hiding this comment.
Don't repeat expressions, plz.
Extract variables.
There was a problem hiding this comment.
This references the for loop iterator, so can't be broken out
| @@ -228,7 +259,6 @@ def main(): | |||
| # Create pipeline which creates infrastructure and deploys services | |||
| print(f"=> Calling `terraform plan --out=terraform.plan`") | |||
| return_code = subprocess.call(["terraform", "plan", "--out=terraform.plan"]) | |||
There was a problem hiding this comment.
Not in this PR.
Please ensure, that terraform python API doesn't satisfy our needs.
There was a problem hiding this comment.
I tried the terraform-python package and it was garbage
