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

Containerise (low priority) #180

Merged
merged 15 commits into from
Jan 3, 2023
Merged

Containerise (low priority) #180

merged 15 commits into from
Jan 3, 2023

Conversation

MattWellie
Copy link
Collaborator

@MattWellie MattWellie commented Dec 23, 2022

Fixes

  • Closes Make an AIP image, instead of running installs #179

  • Creates a Dockerfile, a reasonably extensive .dockerignore, and a github yaml to run the container building process.

  • Updates bumpversion conf to update the dockerfile contents

  • AFAIK this repo has secrets enabled but no runner to execute, so nothing will be built yet the runner(s) are obviously set up for CI, Matt is just an idiot

I'd like to discuss how cpg_workflows is built as a comparison - I'm happy either building only from main, or on deliberate invocations of the build action, rather than with every commit.

I've added a secondary build file that will build an explicitly test version of the image, and will only run when manually invoked

Checklist

  • Related Issue created
  • Tests covering new change
  • Linting checks pass

@vladsavelyev
Copy link
Contributor

Looks good! Manual dispatch for test builds is a good alternative (automated builds on every commit like for cpg_workflows also works good).

There is a lot of documentation and formatting change in this PR besides dockerisation, is this intentional? Would be easier to split documentation and code changes into separate PRs.

@MattWellie MattWellie changed the base branch from main to redraft_documentation December 26, 2022 06:09
@MattWellie
Copy link
Collaborator Author

There is a lot of documentation and formatting change in this PR besides dockerisation, is this intentional?

Nope, that wasn't intentional! I've changed the base branch now (I'd like the docker changes and corresponding documentation changes to be in main at the same time... but not important)

gcloud auth configure-docker australia-southeast1-docker.pkg.dev
- name: build
run: |
docker build . -f Dockerfile --tag $BASE_IMAGE:${{ github.sha }}
Copy link
Contributor

Choose a reason for hiding this comment

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

You could build this as whatever locally, not that it super matters anyway - but you're not restricted by the github.sha

@MattWellie MattWellie merged commit 6011f27 into redraft_documentation Jan 3, 2023
@MattWellie MattWellie deleted the containerise branch January 3, 2023 23:08
MattWellie added a commit that referenced this pull request Jan 9, 2023
* Containerise (low priority) (#180)

* redraft docs (ongoing)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make an AIP image, instead of running installs
3 participants