Page MenuHomePhabricator

[services] Tunnelbroker - Fix scripts in Dockerfiles
ClosedPublic

Authored by karol on May 16 2022, 2:57 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 30, 6:23 PM
Unknown Object (File)
Sun, Jun 30, 7:55 AM
Unknown Object (File)
Sun, Jun 30, 7:55 AM
Unknown Object (File)
Sun, Jun 30, 7:54 AM
Unknown Object (File)
Sun, Jun 30, 7:50 AM
Unknown Object (File)
Mon, Jun 24, 3:50 PM
Unknown Object (File)
Sun, Jun 23, 11:15 PM
Unknown Object (File)
Fri, Jun 21, 9:39 PM

Details

Summary

Raised in https://phabricator.ashoat.com/D3988#112748

I mistakenly put run_service.sh instead of run_tests.sh

Test Plan
cd services
yarn test-tunnelbroker-service

This should run tunnelbroker tests

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

karol edited the test plan for this revision. (Show Details)
karol added reviewers: max, ashoat, tomek.
karol retitled this revision from [services] Tunnelbroker - fix scripts in Dockerfile to [services] Fix scripts in Dockerfiles.May 16 2022, 3:01 AM
karol edited the summary of this revision. (Show Details)
karol edited the test plan for this revision. (Show Details)
karol retitled this revision from [services] Fix scripts in Dockerfiles to [services] Tunnelbroker - Fix scripts in Dockerfiles.
karol edited the summary of this revision. (Show Details)
karol edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.May 16 2022, 6:52 AM
ashoat requested changes to this revision.May 16 2022, 9:42 AM

Just a question: in D3988 you said "[[ ]] doesn't work" – can you clarify why?

This revision now requires changes to proceed.May 16 2022, 9:42 AM

I don't know. For blob and backup I get:

blob-server_1          | /bin/sh: 1: [[: not found
blob-server_1          | Server listening
backup-server_1        | /bin/sh: 1: [[: not found
backup-server_1        | Server listening

So it kind of works but this error message doesn't look good. Is this really worth spending time on and investigating why on earth bash again does weird stuff with the simplest stuff?

ashoat requested changes to this revision.May 18 2022, 11:28 AM

Yes, I think it matters for two reasons:

  1. We've had ENG-157: Make brackets consistent in bash scripts in services open for a while
  2. We should understand what is going on with this stuff and how it's run

So it kind of works but this error message doesn't look good. Is this really worth spending time on and investigating why on earth bash again does weird stuff with the simplest stuff?

My guess is that this isn't bash... otherwise, it would have [[. It took me <2 min of Googling to find this:

On Ubuntu systems, /bin/sh is dash, not bash, and dash does not support the double bracket keyword (or didn't at the time of this posting, I haven't double-checked).

Honestly, it probably is taking me longer to type out this essay for you than it would have taken you to simply research the issue, as requested.

This is another example of you wasting your reviewers time. I should be able to send you "can you clarify why?" and get a response back, without you re-requesting review.

This revision now requires changes to proceed.May 18 2022, 11:28 AM

I then did some further research (that @karol-bisztyga should have done) to understand how CMD works. Here is what I found:

When using the shell form, the specified binary is executed with an invocation of the shell using /bin/sh -c. You can see this clearly if you run a container and then look at the docker ps output

So it seems we should consider CMD /bin/bash -c here.

Raising this diff because D4751 was created as a duplicate and we need to fix this in a Tunnelbroker to run the tests from the CI.
We already have the same lines in a backup and blob Docker files. It's tested and works as expected.
This is an old diff and from that time @ashoat needs to be removed from reviewers and I think @tomek should be as blocking or I can commandeer this diff.

/bin/sh: 1: [[: not found

Ubuntu uses dash for /bin/sh which is specifically not BASH compatible, it's just meant to be a fast UNIX compatible shell

# ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Jul 18  2019 /bin/sh -> dash

https://wiki.archlinux.org/title/Dash

This revision is now accepted and ready to land.Aug 8 2022, 5:58 PM