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

[docker] remove (redundant) chmod and testable workflows #1710

Merged
merged 3 commits into from
Nov 27, 2023

Conversation

TrevorBenson
Copy link
Collaborator

@TrevorBenson TrevorBenson commented Nov 26, 2023

Description

  • Removes the redundant chown from step 12, relying on the final one in step 20.
  • Adds new workflow inputs
    • guild_deploy_branch which defaults to master
    • testing which defaults to false
  • Conditional logic on the push operation
    • If branch is master and testing is false, the push will occur.
    • If branch is master but testing is true, the push will not occur
    • If branch is NOT master, but testing is falase, the push will not occur (prevents mistaken test from updating the public images).

Where should the reviewer start?

Compare the two builds. Try a workflow dispatch with testing true, and with branch not set to master.

Motivation and context

An error in build process when no shell scripts exist in /home/guild/.scripts/ when the operation occurs. Allows testing of the branches for docker building similar to pre-merge testing of changes to guild-deploy.sh etc.

Which issue it fixes?

Closes #1709

How has this been tested?

Local build, and workflow job

@TrevorBenson TrevorBenson self-assigned this Nov 26, 2023
@TrevorBenson TrevorBenson requested a review from rdlrt November 26, 2023 16:39
@TrevorBenson TrevorBenson changed the title remove (redundant) chmod in step 12, keep chmod in step 20 [docker] remove (redundant) chmod in step 12, keep chmod in step 20 Nov 26, 2023
@TrevorBenson TrevorBenson added bug Something isn't working docker labels Nov 26, 2023
@TrevorBenson
Copy link
Collaborator Author

TrevorBenson commented Nov 26, 2023

I didn't dig deep enough in the prior PR #1707 to notice this wasn't Ogmios-related, and it was dropped along with the Ogmios change. Unsure which actual commit it comes from as I have a container built from the #1676 PR. So I (or possibly someone else) may have introduced it in a later commit.

Once the Ogmios change merges into master I'll use the workflow to confirm builds are working again for new Pull Requests. Otherwise, I may open a PR for the workflows that use a default master branch but allow a workflow dispatch to target the alpha branch as well to test prior to things merging into the master, as the current build logic makes it easy to run a test which appears to work but could be due to targetting master and not alpha.

@TrevorBenson TrevorBenson changed the title [docker] remove (redundant) chmod in step 12, keep chmod in step 20 [docker] remove (redundant) chmod and testable workflows Nov 26, 2023
@TrevorBenson
Copy link
Collaborator Author

I went ahead and merged the workflow inputs and logic into this branch, it made it easier to test everything together, confirm a failure on alpha and then the success on container-chown-error, the branch of this PR.

@rdlrt rdlrt merged commit 30704c1 into alpha Nov 27, 2023
2 checks passed
@rdlrt rdlrt deleted the container-chown-error branch November 27, 2023 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Container build fails due to no such file or directory.
2 participants