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

refactor(ci): use docker instead of Konlet for GCP deployments in CI #4252

Merged
merged 29 commits into from
May 3, 2022

Conversation

gustavovalverde
Copy link
Member

@gustavovalverde gustavovalverde commented Apr 30, 2022

Motivation

This is a workaround for an issue related to disk partitioning, caused by a GCP service called Konlet, while mounting the cached disks to the VM and then to the container.

This has been causing most integration tests to fail when a disk with cached state is needed.

Fixes #4114
Closes #4080
Closes #4090
Depends-On: #4266

Solution

  • Do not run our tests directly by GCP services, instead manually create a container inside the VM with docker

** Drawbacks **

  • We won't have the container logs available in GCP, but this is not as important as having the logs available in GitHub Actions, and we're not yet using GCP for reporting.

Review

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

Follow up with GCP support ticket to confirm if there's a fix or an unknown workaround which doesn't require us to stop using Konlet.

This is a workaround for an issue related to disk partitioning, caused by a GCP service called Konlet, while mounting the cached disks to the VM and then to the container
@gustavovalverde gustavovalverde added C-bug Category: This is a bug A-infrastructure Area: Infrastructure changes A-devops Area: Pipelines, CI/CD and Dockerfiles P-Critical 🚑 I-integration-fail Continuous integration fails, including build and test failures lightwalletd any work associated with lightwalletd labels Apr 30, 2022
@gustavovalverde gustavovalverde self-assigned this Apr 30, 2022
@gustavovalverde gustavovalverde marked this pull request as ready for review May 2, 2022 21:32
@gustavovalverde gustavovalverde requested review from a team as code owners May 2, 2022 21:32
@gustavovalverde gustavovalverde requested review from dconnolly and removed request for a team May 2, 2022 21:32
@gustavovalverde gustavovalverde added do-not-merge Tells Mergify not to merge this PR and removed do-not-merge Tells Mergify not to merge this PR labels May 2, 2022
@gustavovalverde
Copy link
Member Author

gustavovalverde commented May 2, 2022

I've confirm we'll have to use this workaround. The solutions given by GCP's support (which fixed other issues) didn't fix this one.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

This is a large PR, which I'm having trouble understanding and reviewing.

It looks like the diff contains:

  • changes from previous PRs in this series
  • bug fixes
  • functional changes
  • syntax and name cleanups

I'd really like to see:

  • a list of changes in this PR
  • how you've tested it
  • test logs that show that we haven't accidentally skipped any tests

If it's not possible to test it until it is merged to main, can we just fix one workflow or disk first? Then we can fix the other workflows once we know the fix works.

@gustavovalverde
Copy link
Member Author

@teor2345 I've removed everything that's not related to this PR

@gustavovalverde
Copy link
Member Author

After merging

This is the most relevant PR to get lightwalletd CI working

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Alright, let's try this and see how it goes.

mergify bot added a commit that referenced this pull request May 3, 2022
@mergify mergify bot merged commit 9b9578c into main May 3, 2022
@mergify mergify bot deleted the ci-gcp-disk branch May 3, 2022 02:47
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-infrastructure Area: Infrastructure changes C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures lightwalletd any work associated with lightwalletd
Projects
None yet
3 participants