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: Add workflows to push, pull, and validate docker images #1676

Merged
merged 34 commits into from
Jul 25, 2023

Conversation

nasdf
Copy link
Member

@nasdf nasdf commented Jul 20, 2023

Relevant issue(s)

Resolves #1674
Resolves #1266

Description

This PR adds a GitHub Action workflow that builds and publishes DefraDB container images to DockerHub & GitHub container registries.

Push Docker images to registries Job:
https://github.com/sourcenetwork/defradb/actions/runs/5651002188

Pull Docker image Job:
https://github.com/sourcenetwork/defradb/actions/runs/5651962572

Validate containerfile job:
https://github.com/sourcenetwork/defradb/actions/runs/5652056549

GitHub Container Image:
https://github.com/sourcenetwork/defradb/pkgs/container/defradb

DockerHub Container Image:
https://hub.docker.com/repository/docker/sourcenetwork/defradb/general

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Manual

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

  • MacOS

@nasdf nasdf added the ci/build This is issue is about the build or CI system, and the administration of it. label Jul 20, 2023
@nasdf nasdf self-assigned this Jul 20, 2023
@nasdf nasdf marked this pull request as draft July 20, 2023 22:11
@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02% ⚠️

Comparison is base (f4670dc) 75.32% compared to head (d30d761) 75.30%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1676      +/-   ##
===========================================
- Coverage    75.32%   75.30%   -0.02%     
===========================================
  Files          208      208              
  Lines        21698    21698              
===========================================
- Hits         16343    16339       -4     
- Misses        4211     4215       +4     
  Partials      1144     1144              
Flag Coverage Δ
all-tests 75.30% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4670dc...d30d761. Read the comment docs.

@nasdf nasdf marked this pull request as ready for review July 20, 2023 22:27
@nasdf nasdf requested a review from shahzadlone July 20, 2023 22:27
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.

It seems good to me but I'll let Shahzad approve it as he's much better at CI stuff than I am.

@fredcarle
Copy link
Collaborator

Not sure if you saw this but this draft PR was trying to do something similar #1350.
The associated issue is #1266.

@nasdf
Copy link
Member Author

nasdf commented Jul 21, 2023

Not sure if you saw this but this draft PR was trying to do something similar #1350. The associated issue is #1266.

My bad, I should have searched for that before starting. I'll reference that open issue here as well.

@shahzadlone
Copy link
Member

shahzadlone commented Jul 21, 2023

Hey Keenan let me know when this is unblocked as I still see PR description says blocked until secrets are added (are the secrets not working?)

Please link the last execution run of this actions successfull execution in the PR.

Would be nice to also see the link to the registery when the build gets pushed to.

@nasdf
Copy link
Member Author

nasdf commented Jul 21, 2023

Hey Keenen let me know when this is unblocked as I still see PR description says blocked until secrets are added (are the secrets not working?)

Please link the last execution run of this actions successfull execution in the PR.

Would be nice to also see the link to the registery when the build gets pushed to.

My bad, I forgot to remove that after you added the secrets. I'll attempt a test run of it now.

@shahzadlone
Copy link
Member

suggestion: You could try testing two ways:

  1. try pull_request_target
  2. try opening a dummy draft pr with branch flow instead of forkflow

Note: secrets will be available with tag push

@nasdf nasdf requested a review from shahzadlone July 21, 2023 21:40
.github/workflows/build-then-deploy-docker.yml Outdated Show resolved Hide resolved
.github/workflows/build-then-deploy-docker.yml Outdated Show resolved Hide resolved
.github/workflows/build-then-deploy-docker.yml Outdated Show resolved Hide resolved
.github/workflows/build-then-deploy-docker.yml Outdated Show resolved Hide resolved
.github/workflows/build-then-deploy-docker.yml Outdated Show resolved Hide resolved
Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

Some thoughts on the new approach:

We want 2 things on top of the previous basic implementation which did the basic build and push:

  1. We wanted to make sure our container file is always in a valid state (it is possible to be in a valid state but pushes something that is missing deps, as we had with the shared lib recently).

  2. We want to test that we can pull the pushed docker image (from either one of the registry should suffice) and attempt to run the binary, to ensure there are no error like we had with the shared lib previously.

I see both 1) and 2) as separate features from each other and from what we had previously.

Problem with the current approach:

  • It combines the pushing of the image and validating the container file (1) in one step, meaning we will only be able to validate the container file on every tag-push (IMO it needs to be in valid state always). Here is an idea for a separate workflow that will run every time the container file or workflow file is touched in a pr + on every push to master/develop.
# File:  defradb/.github/workflows/validate-containerfile.yml
# Copyright 2023 Democratized Data Foundation
# INSERT REST OF HEADER

name: Validate Containerfile Workflow

on:
  pull_request:
    branches:
      - master
      - develop
    paths:
      - '.github/workflows/validate-containerfile.yml'
      - 'tools/defradb.containerfile'

  push:
    branches:
      - master
      - develop

jobs:
  validate-containerfile:
    name: Validate containerfile job

    runs-on: ubuntu-latest

    steps: # Insert the steps to validate the containerfile

Note: You can also keep the current new validation step you added to additionally check before pushing as well (as it still will only build once for each platform and reuse the build while pushing).

As for (2) you can have an action spin of after this action is completed automatically if you do something like this:

# File:  defradb/.github/workflows/pull-docker-image.yml

# Copyright 2023 Democratized Data Foundation
# INSERT REST OF HEADER

name: Pull Docker Image Workflow

on:
  workflow_run:
    # Warning: this workflow must NOT:
    # - interact with any new code.
    # - checkout new code.
    # - build/compile anything (only pull).
    # - make any indirect calls (i.e. make xyz, or npm install, etc.)
    # Note this workflow:
    # - will use the base's (or default) workflow file's state.
    # - doesn't run on the PR or the branch coming in, it runs on the default branch.
    # - has read-write repo token
    # - has access to secrets
    workflows: ["Push Docker Image To Registries Workflow"]
    types:
      - completed

jobs:
  pull-docker-image:
    name: Pull docker image job

    if: ${{ github.event.workflow_run.conclusion == 'success' }}

    runs-on: ubuntu-latest

    steps: #INSERT STEPS 

@nasdf nasdf requested a review from shahzadlone July 25, 2023 00:40
@nasdf
Copy link
Member Author

nasdf commented Jul 25, 2023

Note: You can also keep the current new validation step you added to additionally check before pushing as well (as it still will only build once for each platform and reuse the build while pushing).

I attempted to keep this functionality but I ran into an issue with Docker not allowing export of multi platform images.

@shahzadlone
Copy link
Member

Note: You can also keep the current new validation step you added to additionally check before pushing as well (as it still will only build once for each platform and reuse the build while pushing).

I attempted to keep this functionality but I ran into an issue with Docker not allowing export of multi platform images.

Lets just stick to default platform for now. We can look into adding more platforms later then.

Lmk if you need any clarification on the other 2 workflows i mentioned.

@@ -9,11 +9,16 @@ WORKDIR /repo/
COPY go.mod go.sum Makefile ./
RUN make deps:modules
Copy link
Member

Choose a reason for hiding this comment

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

thought: Maybe we can get by not building lens deps here as they might be only dev dependencies. I will make a ticket and it can be taken care of outside the scope of this PR.
New Issue here:
#1707

Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

LGTM! This is some awesome work, appreciate you adding all the changes just shared one thought and still wondering if you are doing the one nitpick I raised before (but is non-blocking).

nitpick: #1676 (comment)

@shahzadlone
Copy link
Member

shahzadlone commented Jul 25, 2023

nitpick: Can update title to ci: Add workflows to push, pull & validate docker or anything else to indicate that there are multiple workflows being added now. I am not sure if we need an i because seems like something that might be useful to see in changelog.

@nasdf nasdf changed the title ci: Add GitHub Action workflow for Docker deploy ci: Add workflows to push, pull, and validate docker images Jul 25, 2023
@nasdf nasdf merged commit 3061bf0 into sourcenetwork:develop Jul 25, 2023
13 checks passed
@nasdf nasdf deleted the nasdf/feat/ci-docker-deploy branch July 25, 2023 04:04
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…twork#1676)

## Relevant issue(s)

Resolves sourcenetwork#1674 
Resolves sourcenetwork#1266

## Description

This PR adds a GitHub Action workflow that builds and publishes DefraDB
container images to DockerHub & GitHub container registries.

Push Docker images to registries Job:
https://github.com/sourcenetwork/defradb/actions/runs/5651002188

Pull Docker image Job:
https://github.com/sourcenetwork/defradb/actions/runs/5651962572

Validate containerfile job:
https://github.com/sourcenetwork/defradb/actions/runs/5652056549

GitHub Container Image:
https://github.com/sourcenetwork/defradb/pkgs/container/defradb

DockerHub Container Image:
https://hub.docker.com/repository/docker/sourcenetwork/defradb/general

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Manual

Specify the platform(s) on which this was tested:
- MacOS
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker container CI Workflow to publish container to public registy
3 participants