First commits #5

king-alexander · 2023-02-14T18:58:42Z

🗣 Description

These are the first commits towards a repository decomposition for the Admiral. They generally pull in Admiral's Docker components and change defaults inherited from the skeleton repository.

💭 Motivation and context

It's desirable to separate the containerization logic from a project's content. This pull request is a step towards decomposing Admiral into smaller, more manageable repositories.

Closes Replace Celery Flower Image #4: I used the opportunity to pick off some low-hanging fruit. The mher/flower image was last updated 3 months ago.
Resolves Separate Docker components admiral#40

🧪 Testing

I modified the standard tests inherited from cisagov/skeleton-docker to cover my changes to the Docker configuration. Not all tests were easy to modify. In particular, the main_container tests do not transfer well, as there is no main container in the Docker composition. Implementing these tests is scoped as future work in #6.

✅ Pre-approval checklist

This PR has an informative and human-readable title.
Changes are limited to a single goal - eschew scope creep!
All future TODOs are captured in issues, which are referenced
in code comments.
All relevant type-of-change labels have been added.
I have read the CONTRIBUTING document.
These code changes follow cisagov code standards.
All relevant repo and/or project documentation has been updated
to reflect the changes in this PR.
Tests have been added and/or modified to cover the changes in this PR.
All new and existing tests pass.

✅ Pre-merge checklist

Revert dependencies to default branches.
Finalize version.

✅ Post-merge checklist

Add a tag or create a release.

dav3r · 2023-02-24T18:03:09Z

FYI, the linting failures (here and here) should be resolved after #7 has been approved and merged into this branch.

mcdonnnj

Before you make any other changes please update the image name:

admiral-docker/.github/workflows/build.yml

Line 30 in 93969b2

IMAGE_NAME: cisagov/example

Dockerfile

dav3r · 2023-02-24T18:56:04Z

@king-alexander Now that #7 has been merged, you should pull those changes in to this branch and deconflict them. Before you do that though, make sure you do what @mcdonnnj said in his comment. That should get you closer to passing all of the checks in this PR.

Dockerfile

dav3r · 2023-02-24T19:51:56Z

Before you make any other changes please update the image name:

admiral-docker/.github/workflows/build.yml

Line 30 in 93969b2

IMAGE_NAME: cisagov/example

BTW, per our convention, the image should be called admiral, not admiral-docker.

dav3r

Noting a few more items:

README.md

In other words, direct this Dockerfile to install source code and dependencies for the admiral python library.

Here we change the entrypoint to "admiral" and remove various instructions this image doesn't need.

Define the secrets that this Docker composition uses (and remove the one it doesn't).

king-alexander · 2023-03-16T14:00:07Z

I dropped support for all platforms except linux/amd64 and linux/arm64 in 58ba017. They should be reenabled after https://github.com/cisagov/skeleton-docker is updated to address the rust compiler issue.

.github/workflows/build.yml

dav3r

I think this is looking real solid now - nice work! 🤠

jsf9k

I have one minor question.

Dockerfile

jsf9k

Strong work!

dav3r · 2023-04-14T18:22:55Z

@mcdonnnj I believe this PR is just waiting on your re-review.

jsf9k · 2023-08-17T14:43:33Z

@mcdonnnj - Why is Lineage not generating a PR for this repo? The develop branch is missing the latest commits from the skeleton.

mcdonnnj · 2023-08-18T18:37:19Z

@king-alexander Please either rebase on or merge develop now that #12 has been merged. This should put you in position to pass tests with this PR.

This commit instantiates a Docker client with the Docker Compose file path and the project name. It also comments out the main container configuration, which causes tests to fail per #6.

Uses the python_on_whales syntax and comments out test_wait_for_exits, which requires a main container.

I made the requested change and asked Nick to re-review as time permits. If he has more suggestions, I will address them in a new pull request.

king-alexander added documentation This issue or pull request improves or adds to documentation docker Pull requests that update Docker code labels Feb 14, 2023

king-alexander mentioned this pull request Feb 16, 2023

Implement standard container tests #6

Open

2 tasks

king-alexander force-pushed the first-commits branch 4 times, most recently from cdc82f2 to 93969b2 Compare February 23, 2023 17:17

king-alexander marked this pull request as ready for review February 23, 2023 17:28

king-alexander requested review from dav3r, felddy, jsf9k and mcdonnnj as code owners February 23, 2023 17:28

mcdonnnj previously requested changes Feb 24, 2023

View reviewed changes

dav3r mentioned this pull request Feb 24, 2023

Lineage pull request for: skeleton #7

Merged

dav3r requested changes Feb 24, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

dav3r reviewed Feb 24, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

dav3r requested changes Feb 24, 2023

View reviewed changes

king-alexander force-pushed the first-commits branch from b247055 to 759d3a9 Compare March 2, 2023 15:23

king-alexander added 5 commits March 2, 2023 09:46

Specify admiral repository

d5595fe

In other words, direct this Dockerfile to install source code and dependencies for the admiral python library.

Change the image author

9fecb6a

Upgrade Python version

5cd29d0

Define Admiral environment variables

b36174e

Modify run preparations

b8a2642

Here we change the entrypoint to "admiral" and remove various instructions this image doesn't need.

king-alexander force-pushed the first-commits branch from 904fc2d to 3028049 Compare March 2, 2023 16:01

king-alexander added 3 commits March 2, 2023 10:10

Define secrets

2246327

Define the secrets that this Docker composition uses (and remove the one it doesn't).

Define admiral template

b4354ad

Define admiral services

7a1da24

king-alexander force-pushed the first-commits branch from 88499e9 to 88cd16a Compare March 9, 2023 18:45

dav3r reviewed Mar 16, 2023

View reviewed changes

.github/workflows/build.yml Show resolved Hide resolved

king-alexander mentioned this pull request Mar 21, 2023

Reintroduce support for dropped platforms #8

Open

1 task

Modify supported platforms

6dc81cf

king-alexander force-pushed the first-commits branch from 58ba017 to 6dc81cf Compare March 21, 2023 16:49

king-alexander requested a review from dav3r March 21, 2023 17:26

dav3r approved these changes Mar 22, 2023

View reviewed changes

jsf9k reviewed Mar 22, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

king-alexander self-assigned this Mar 22, 2023

Update Python version

d81a6d8

jsf9k approved these changes Mar 23, 2023

View reviewed changes

jsf9k mentioned this pull request Jul 14, 2023

Lineage pull request for: skeleton #11

Merged

2 tasks

king-alexander mentioned this pull request Jul 21, 2023

Deploy Admiral cisagov/cyhy_amis#639

Open

king-alexander mentioned this pull request Jul 28, 2023

Implement functional testing cisagov/admiral#9

Open

Merge branch 'develop' into first-commits

15c1431

king-alexander requested a review from jasonodoom as a code owner August 17, 2023 14:26

dav3r mentioned this pull request Aug 18, 2023

Lineage pull request for: skeleton #12

Merged

2 tasks

Merge branch 'develop' into first-commits

1947eb9

king-alexander force-pushed the first-commits branch from 7490071 to 1947eb9 Compare August 21, 2023 21:47

king-alexander added 4 commits August 21, 2023 16:48

Bump version from 1.3.0 to 1.4.0

cfd3036

Configure python_on_whales pytest plugin

0dbd4a3

This commit instantiates a Docker client with the Docker Compose file path and the project name. It also comments out the main container configuration, which causes tests to fail per #6.

Update container tests

5d7e00a

Uses the python_on_whales syntax and comments out test_wait_for_exits, which requires a main container.

Bump Dockerfile version

314bf56

king-alexander merged commit d69c24f into develop Aug 22, 2023
10 checks passed

king-alexander deleted the first-commits branch August 22, 2023 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First commits #5

First commits #5

king-alexander commented Feb 14, 2023 •

edited

Loading

dav3r commented Feb 24, 2023

mcdonnnj left a comment •

edited

Loading

dav3r commented Feb 24, 2023

dav3r commented Feb 24, 2023 •

edited

Loading

dav3r left a comment

king-alexander commented Mar 16, 2023

dav3r left a comment •

edited

Loading

jsf9k left a comment

jsf9k left a comment

dav3r commented Apr 14, 2023

jsf9k commented Aug 17, 2023

mcdonnnj commented Aug 18, 2023

First commits #5

First commits #5

Conversation

king-alexander commented Feb 14, 2023 • edited Loading

🗣 Description

💭 Motivation and context

🧪 Testing

✅ Pre-approval checklist

✅ Pre-merge checklist

✅ Post-merge checklist

dav3r commented Feb 24, 2023

mcdonnnj left a comment • edited Loading

Choose a reason for hiding this comment

dav3r commented Feb 24, 2023

dav3r commented Feb 24, 2023 • edited Loading

dav3r left a comment

Choose a reason for hiding this comment

king-alexander commented Mar 16, 2023

dav3r left a comment • edited Loading

Choose a reason for hiding this comment

jsf9k left a comment

Choose a reason for hiding this comment

jsf9k left a comment

Choose a reason for hiding this comment

dav3r commented Apr 14, 2023

jsf9k commented Aug 17, 2023

mcdonnnj commented Aug 18, 2023

king-alexander commented Feb 14, 2023 •

edited

Loading

mcdonnnj left a comment •

edited

Loading

dav3r commented Feb 24, 2023 •

edited

Loading

dav3r left a comment •

edited

Loading