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/CD breakage #283

Closed
HalosGhost opened this issue Jul 17, 2024 · 6 comments · Fixed by #285
Closed

CI/CD breakage #283

HalosGhost opened this issue Jul 17, 2024 · 6 comments · Fixed by #285

Comments

@HalosGhost
Copy link
Collaborator

@rockett-m, it looks like the docker merge CI/CD step is failing from the merge of #265. It's not spurious (rerunning the job failed as well). Help resolving would be appreciated.

@maurermi
Copy link
Collaborator

@rockett-m, it looks like the docker merge CI/CD step is failing from the merge of #265. It's not spurious (rerunning the job failed as well). Help resolving would be appreciated.

It looks like the failure occurs when running install-build-tools.sh in docker merge, and it appears that the docker merge job uses sh instead of bash. I think the problem is now we are calling source in this script which causes the error since source is not supported by sh.

(Sending this from my phone, will verify further when able)

@rockett-m
Copy link
Contributor

rockett-m commented Jul 18, 2024

Just got online. Michael, that sounds logical.

Would it be ok to add shell: bash to the parameters for the docker merge CI/CD step? inside .github/workflows/docker-merge.yml

defaults:
  run:
    shell: bash

I'll set that preference and try with act -j docker-build --container-architecture linux/amd64 to see if that runs the source correctly and passes.

@maurermi
Copy link
Collaborator

I am not sure of an argument for using sh instead of bash, so I do not see a problem with this (@HalosGhost you may have a more nuanced opinion here)

@HalosGhost
Copy link
Collaborator Author

Out of a preference for portability, I'd probably be happiest if we completely ripped out bash and relied purely on POSIX sh, but I don't think that's really a reasonable standard given how constrained our support matrix is right now anyway. Plus, bash is already pretty intertwined as a dep.

I'm entirely fine with our workflows requiring bash. Having said that, the POSIX spelling of source is ., and we could reasonably do that instead (as bash is POSIX-compatible, . works in it as well—same for zsh and any other POSIX-compatible shell).

@rockett-m
Copy link
Contributor

That's a good idea with the '.' swapped in. I'll give it a go.

In our ci.yml we have the shell defaults set to bash so that's why I suggested it initially.

@rockett-m
Copy link
Contributor

rockett-m commented Jul 18, 2024

I think the issue could be related to the Dockerfile where the venv file should be included so it can be found by the CI/CD docker-merge...will continue testing. Note - that alone does not resolve the issue.

COPY scripts/install-build-tools.sh /opt/tx-processor/scripts/install-build-tools.sh
COPY scripts/setup-dependencies.sh /opt/tx-processor/scripts/setup-dependencies.sh
COPY scripts/activate-venv.sh /opt/tx-processor/scripts/activate-venv.sh

traced from here:

**| 249.5 scripts/install-build-tools.sh: line 89: scripts/activate-venv.sh: No such file or directory**
| ------
| Dockerfile:23
| --------------------
|   21 |     WORKDIR /opt/tx-processor
|   22 |     
|   23 | >>> RUN /usr/bin/env bash -c scripts/install-build-tools.sh
|   24 |     RUN scripts/setup-dependencies.sh
|   25 |     
| --------------------

rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 18, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
@HalosGhost HalosGhost linked a pull request Jul 18, 2024 that will close this issue
rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 18, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 19, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 19, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 19, 2024
Co-authored-by: Michael Maurer <maurer.mi@northeastern.edu>

Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 19, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 19, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
rockett-m added a commit to rockett-m/opencbdc-tx that referenced this issue Jul 19, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
HalosGhost pushed a commit that referenced this issue Jul 19, 2024
Signed-off-by: Morgan Rockett <morgan.rockett@tufts.edu>
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 a pull request may close this issue.

3 participants