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

ci: Fix terraform plan and merge AMI build + deploy workflow #1514

Merged
merged 4 commits into from
May 18, 2023

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented May 17, 2023

Relevant issue(s)

Resolves #1516

Description

Issue: #1516 Has a good overview of the problem, and the top 3 goals outlined.

  • AMI Building and Deploying to EC2 instance is now 2 jobs in one workflow rather than split across workflow_run that would previously not show in checks, nor run on the same commit / branch (as workflow_run runs on develop branch only).
  • Fixes the broken terraform plan action use that wouldn't comment on the PR.
  • The fixed terraform plan action happens only on PRs that are on base of master or develop and changes the files which affect the AMI building and deployment (terraform files, packer files, relevant GitHub workflow files).

Note:

  • Changing these AMI building and deployment files (terraform, packer, workflows) should always happen from a branch-flow (not from the fork-flow), So that secrets are always available.
  • The terraform plan workflow can't be made required as it only triggers when the ami related files are changed, if we made it required it will always be waiting for it to run even in the cases we don't expect that workflow to run.

How has this been tested?

  • Tested from fork-flow, branch-flow, manual triggers, and with and without secrets.
  • Both comment cases introduced in workflows are posted in the PR through triggering those cases.

Specify the platform(s) on which this was tested:

  • WSL2 Manjaro

@codecov
Copy link

codecov bot commented May 17, 2023

Codecov Report

Merging #1514 (7799475) into develop (8c3984d) will not change coverage.
The diff coverage is n/a.

❗ Current head 7799475 differs from pull request most recent head 8f71577. Consider uploading reports for the commit 8f71577 to get more accurate results

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1514   +/-   ##
========================================
  Coverage    72.06%   72.06%           
========================================
  Files          185      185           
  Lines        18295    18295           
========================================
  Hits         13184    13184           
  Misses        4066     4066           
  Partials      1045     1045           

see 4 files with indirect coverage changes

@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch 4 times, most recently from 981f69d to 26d9b05 Compare May 17, 2023 20:46
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from 26d9b05 to ed1602d Compare May 17, 2023 20:47
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from 5f5d3a3 to 3e79f6f Compare May 17, 2023 21:29
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from 3e79f6f to 30fd0fc Compare May 17, 2023 21:44
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from 1ffb2cc to 5488417 Compare May 17, 2023 22:08
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@sourcenetwork sourcenetwork deleted a comment from source-devs May 17, 2023
@shahzadlone shahzadlone requested a review from a team May 18, 2023 01:14
@shahzadlone shahzadlone self-assigned this May 18, 2023
@shahzadlone shahzadlone added ci/build This is issue is about the build or CI system, and the administration of it. code quality Related to improving code quality labels May 18, 2023
@shahzadlone shahzadlone changed the title temp: CI TESTING of packer and terraform actions ci: fix terraform plan & merge AMI build + AMI deploy workflow May 18, 2023
@shahzadlone shahzadlone changed the title ci: fix terraform plan & merge AMI build + AMI deploy workflow ci: fix terraform plan and merge AMI build + deploy workflow May 18, 2023
@shahzadlone shahzadlone changed the title ci: fix terraform plan and merge AMI build + deploy workflow ci: Fix terraform plan and merge AMI build + deploy workflow May 18, 2023
@shahzadlone shahzadlone marked this pull request as ready for review May 18, 2023 01:22
@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from 5488417 to a46497a Compare May 18, 2023 01:32
@sourcenetwork sourcenetwork deleted a comment from source-devs May 18, 2023
@source-devs
Copy link

Warning: you made changes to files that require privileged access, this means you are either using the fork-flow, or are missing some secrets.

Solution: please use branch-flow, or add the missing secrets. If you are not an internal developer, please reach out to a maintainer for assistance.

Note: the files that were changed also require manual testing using our organization AWS account, and using manual triggers on some of our workflows (that are not triggered normally).

Pushed by: @shahzadlone, SHA: a46497a630fc14b3e7a7cbacec50b38d25fad52e

@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from a46497a to a8ce0f8 Compare May 18, 2023 01:35
@source-devs
Copy link

Terraform Format and Style success

Terraform Initialization success

Terraform Validation success

Terraform Plan success

Show Plan

Terraform Plan Output:
data.aws_ami.ami: Reading...
aws_security_group.sg: Refreshing state... [id=sg-03ba6f1f9cd118f43]
data.aws_ami.ami: Read complete after 0s [id=ami-090df366ae87f905c]
aws_instance.instance: Refreshing state... [id=i-0861cf88407e45be4]

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

# aws_instance.instance must be replaced
-/+ resource "aws_instance" "instance" {
    ~ ami                                  = "ami-0aec60e193aa9f7f0" -> "ami-090df366ae87f905c" # forces replacement
    ~ arn                                  = "arn:aws:ec2:us-east-1:575155546886:instance/i-0861cf88407e45be4" -> (known after apply)
    ~ associate_public_ip_address          = true -> (known after apply)
    ~ availability_zone                    = "us-east-1c" -> (known after apply)
    ~ cpu_core_count                       = 1 -> (known after apply)
    ~ cpu_threads_per_core                 = 1 -> (known after apply)
    ~ disable_api_stop                     = false -> (known after apply)
    ~ disable_api_termination              = false -> (known after apply)
    ~ ebs_optimized                        = false -> (known after apply)
    - hibernation                          = false -> null
    + host_id                              = (known after apply)
    + host_resource_group_arn              = (known after apply)
    + iam_instance_profile                 = (known after apply)
    ~ id                                   = "i-0861cf88407e45be4" -> (known after apply)
    ~ instance_initiated_shutdown_behavior = "stop" -> (known after apply)
    ~ instance_state                       = "running" -> (known after apply)
    ~ ipv6_address_count                   = 0 -> (known after apply)
    ~ ipv6_addresses                       = [] -> (known after apply)
    ~ monitoring                           = false -> (known after apply)
    + outpost_arn                          = (known after apply)
    + password_data                        = (known after apply)
    + placement_group                      = (known after apply)
    ~ placement_partition_number           = 0 -> (known after apply)
    ~ primary_network_interface_id         = "eni-08a3ca5679d535293" -> (known after apply)
    ~ private_dns                          = "ip-172-31-80-89.ec2.internal" -> (known after apply)
    ~ private_ip                           = "172.31.80.89" -> (known after apply)
    ~ public_dns                           = "ec2-18-212-67-74.compute-1.amazonaws.com" -> (known after apply)
    ~ public_ip                            = "18.212.67.74" -> (known after apply)
    ~ secondary_private_ips                = [] -> (known after apply)
    ~ subnet_id                            = "subnet-0bc2e025" -> (known after apply)
      tags                                 = {
          "environment"  = "dev"
          "organization" = "source"
      }
    ~ tenancy                              = "default" -> (known after apply)
    + user_data                            = (known after apply)
    + user_data_base64                     = (known after apply)
    ~ vpc_security_group_ids               = [
        - "sg-03ba6f1f9cd118f43",
      ] -> (known after apply)
      # (7 unchanged attributes hidden)

    ~ capacity_reservation_specification {
        ~ capacity_reservation_preference = "open" -> (known after apply)

        + capacity_reservation_target {
            + capacity_reservation_id                 = (known after apply)
            + capacity_reservation_resource_group_arn = (known after apply)
          }
      }

    - credit_specification {
        - cpu_credits = "standard" -> null
      }

    + ebs_block_device {
        + delete_on_termination = (known after apply)
        + device_name           = (known after apply)
        + encrypted             = (known after apply)
        + iops                  = (known after apply)
        + kms_key_id            = (known after apply)
        + snapshot_id           = (known after apply)
        + tags                  = (known after apply)
        + throughput            = (known after apply)
        + volume_id             = (known after apply)
        + volume_size           = (known after apply)
        + volume_type           = (known after apply)
      }

    ~ enclave_options {
        ~ enabled = false -> (known after apply)
      }

    + ephemeral_block_device {
        + device_name  = (known after apply)
        + no_device    = (known after apply)
        + virtual_name = (known after apply)
      }

    ~ maintenance_options {
        ~ auto_recovery = "default" -> (known after apply)
      }

    ~ metadata_options {
        ~ http_endpoint               = "enabled" -> (known after apply)
        ~ http_put_response_hop_limit = 1 -> (known after apply)
        ~ http_tokens                 = "optional" -> (known after apply)
        ~ instance_metadata_tags      = "disabled" -> (known after apply)
      }

    + network_interface {
        + delete_on_termination = (known after apply)
        + device_index          = (known after apply)
        + network_card_index    = (known after apply)
        + network_interface_id  = (known after apply)
      }

    ~ private_dns_name_options {
        ~ enable_resource_name_dns_a_record    = false -> (known after apply)
        ~ enable_resource_name_dns_aaaa_record = false -> (known after apply)
        ~ hostname_type                        = "ip-name" -> (known after apply)
      }

    ~ root_block_device {
        ~ delete_on_termination = true -> (known after apply)
        ~ device_name           = "/dev/sda1" -> (known after apply)
        ~ encrypted             = false -> (known after apply)
        ~ iops                  = 100 -> (known after apply)
        + kms_key_id            = (known after apply)
        ~ tags                  = {} -> (known after apply)
        ~ throughput            = 0 -> (known after apply)
        ~ volume_id             = "vol-07158d9693e5103ba" -> (known after apply)
        ~ volume_size           = 8 -> (known after apply)
        ~ volume_type           = "gp2" -> (known after apply)
      }
  }

Plan: 1 to add, 0 to change, 1 to destroy.

Changes to Outputs:
~ ec2instance = "i-0861cf88407e45be4" -> (known after apply)
~ ip          = "18.212.67.74" -> (known after apply)

─────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't
guarantee to take exactly these actions if you run "terraform apply" now.


Pushed By: @shahzadlone
SHA: a8ce0f83d23dfb383f193ec22f4d3327c3adfd53

Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM with the caveat that I'm not an expert on the topic.

Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM with the caveat that I'm not an expert on the topic.

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM, but my experience here is quite limited ad so I may miss stuff

This is mostly because I see no good reason to use `workflow_run`
specially because it doesn't show with the checks on github, you
have to manually search and find it under an action running on github,
also `workflow_run` always uses the workflow state of `develop` branch
which can cause some tidious CI debugging sessions and inconsistent
behaviour as we only care about running it on the commit the push
happens on.
@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from a8ce0f8 to 464f19a Compare May 18, 2023 17:41
and changed workflow files concerning aws ami build and deploy (packer and terraform)
files. It will fail the workflow on the PR if an unprivileged flow (or
missing secrets) hits the terraform planning action.
@shahzadlone shahzadlone force-pushed the ci/split-terraform-preview-and-deploy branch from 7799475 to 8f71577 Compare May 18, 2023 18:13
@shahzadlone
Copy link
Member Author

shahzadlone commented May 18, 2023

Heads-up: Changing to now not comment incase of unpriv access as github won't allow posting comments without write perms (i.e. no pr write token will be available to unpriv flows specially on fork-flow with pull_request trigger).

Instead will now show more detailed fail error on ci.

@sourcenetwork sourcenetwork deleted a comment from source-devs May 18, 2023
@sourcenetwork sourcenetwork deleted a comment from source-devs May 18, 2023
@shahzadlone shahzadlone merged commit 76b7c87 into develop May 18, 2023
11 checks passed
@shahzadlone shahzadlone deleted the ci/split-terraform-preview-and-deploy branch May 18, 2023 18:29
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…etwork#1514)

## Relevant issue(s)
Resolves sourcenetwork#1516

## Description
#### Issue: sourcenetwork#1516 Has a good overview of the problem, and the top 3
goals outlined.
- AMI Building and Deploying to EC2 instance is now 2 jobs in one
workflow rather than split across `workflow_run` that would previously
not show in checks, nor run on the same commit / branch (as
`workflow_run` runs on develop branch only).
- Fixes the broken `terraform plan` action use that wouldn't comment on
the PR.
- The fixed `terraform plan` action happens only on PRs that are on base
of `master` or `develop` and changes the files which affect the AMI
building and deployment (terraform files, packer files, relevant GitHub
workflow files).

Note: Changing these AMI building and deployment files (terraform,
packer, workflows) should always happen from a branch-flow (not from the
fork-flow), So that secrets are always available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/build This is issue is about the build or CI system, and the administration of it. code quality Related to improving code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix broken terraform plan, + combine ami deploying job with the ami building job in one workflow
4 participants