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

Rewrite etcd scripts in strict mode #15514

Closed
serathius opened this issue Mar 20, 2023 · 4 comments
Closed

Rewrite etcd scripts in strict mode #15514

serathius opened this issue Mar 20, 2023 · 4 comments

Comments

@serathius
Copy link
Member

What would you like to be added?

Follow up from #15504 (comment)

Strict mode as defined in http://redsymbol.net/articles/unofficial-bash-strict-mode/

List of scripts:

$ find . -name *.sh
./tools/mod/install_all.sh
./tools/local-tester/faults.sh
./tools/local-tester/bridge.sh
./tools/rw-heatmaps/rw-benchmark.sh
./tools/etcd-dump-logs/testdecoder/decoder_wrongoutputformat.sh
./tools/etcd-dump-logs/testdecoder/decoder_correctoutputformat.sh
./tests/integration/fixtures-expired/gencerts.sh
./tests/fixtures/gencerts.sh
./tests/manual/docker-dns/insecure/run.sh
./tests/manual/docker-dns/certs-gateway/run.sh
./tests/manual/docker-dns/certs-gateway/gencerts.sh
./tests/manual/docker-dns/certs-wildcard/run.sh
./tests/manual/docker-dns/certs-wildcard/gencerts.sh
./tests/manual/docker-dns/certs/run.sh
./tests/manual/docker-dns/certs/gencerts.sh
./tests/manual/docker-dns/certs-common-name-auth/run.sh
./tests/manual/docker-dns/certs-common-name-auth/gencerts.sh
./tests/manual/docker-dns/certs-common-name-multi/run.sh
./tests/manual/docker-dns/certs-common-name-multi/gencerts.sh
./tests/manual/docker-dns/certs-san-dns/run.sh
./tests/manual/docker-dns/certs-san-dns/gencerts.sh
./tests/manual/docker-dns-srv/certs-gateway/run.sh
./tests/manual/docker-dns-srv/certs-gateway/gencerts.sh
./tests/manual/docker-dns-srv/certs-wildcard/run.sh
./tests/manual/docker-dns-srv/certs-wildcard/gencerts.sh
./tests/manual/docker-dns-srv/certs/run.sh
./tests/manual/docker-dns-srv/certs/gencerts.sh
./tests/manual/docker-static-ip/certs-metrics-proxy/run.sh
./tests/manual/docker-static-ip/certs-metrics-proxy/gencerts.sh
./tests/manual/docker-static-ip/certs/run.sh
./tests/manual/docker-static-ip/certs/gencerts.sh
./hack/patch/cherrypick.sh
./hack/benchmark/bench.sh
./scripts/build.sh
./scripts/build_lib.sh
./scripts/verify_proto_annotations.sh
./scripts/update_proto_annotations.sh
./scripts/measure-test-flakiness.sh
./scripts/release_mod.sh
./scripts/build_tools.sh
./scripts/codecov_upload.sh
./scripts/build-binary.sh
./scripts/genproto.sh
./scripts/updatebom.sh
./scripts/fuzzing.sh
./scripts/build-release.sh
./scripts/install-marker.sh
./scripts/test.sh
./scripts/fix.sh
./scripts/build-docker.sh
./scripts/update_dep.sh
./scripts/test_lib.sh
./scripts/release.sh
./scripts/verify_genproto.sh
./client/v3/experimental/recipes/grpc_gateway/user_add.sh
./pkg/proxy/fixtures/gencerts.sh

Plus list of run commands in github flows (also use bash in permissive mode):

$ grep -r run: .github/workflows/ 
.github/workflows/measure-test-flakiness.yaml:    - run: "./scripts/measure-test-flakiness.sh"
.github/workflows/coverage.yaml:      run: |
.github/workflows/codeql-analysis.yml:    #- run: |
.github/workflows/release.yaml:    - run: |
.github/workflows/grpcproxy.yaml:    - run: date
.github/workflows/grpcproxy.yaml:      run: |
.github/workflows/govuln.yaml:      - run: date
.github/workflows/govuln.yaml:      - run: go install golang.org/x/vuln/cmd/govulncheck@latest && govulncheck ./...
.github/workflows/tests.yaml:    - run: date
.github/workflows/tests.yaml:      run: |
.github/workflows/fuzzing.yaml:    - run: GOARCH=amd64 CPU=4 make fuzz
.github/workflows/static-analysis.yaml:  run:
.github/workflows/static-analysis.yaml:      - run: make verify
.github/workflows/static-analysis.yaml:      - run: make fix
.github/workflows/e2e.yaml:    - run: date
.github/workflows/e2e.yaml:      run: |
.github/workflows/e2e-arm64.yaml:    - run: date
.github/workflows/e2e-arm64.yaml:      run: |
.github/workflows/tests-arm64.yaml:    - run: date
.github/workflows/tests-arm64.yaml:      run: |
.github/workflows/contrib.yaml:    - run: make -C contrib/mixin tools test
.github/workflows/robustness-template.yaml:      run: |
.github/workflows/robustness-template.yaml:      run: |
.github/workflows/build.yaml:        run: |

Why is this needed?

Should make bash script more reliable and debuggable and let us remove workarounds like suffixing test runs with 2>&1 | tee test.log ! egrep "(--- FAIL:|DATA RACE|panic: test timed out|appears to have leaked)" -B50 -A10 test.log which is horrible.

fuweid added a commit to fuweid/etcd that referenced this issue Mar 20, 2023
According to Github Actions doc [1], linux platform will use `bash -e
{0}` mode to run script. But we also need `pipefail` and `nounset` in
CI. This patch is aimed to add strict mode setting before any run.

And remove the `| tee` and egrep commands since we don't need this
workaround to show error.

REF:

1: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell

ISSUE: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 20, 2023
According to Github Actions doc [1], linux platform will use `bash -e
{0}` mode to run script. But we also need `pipefail` and `nounset` in
CI. This patch is aimed to add strict mode setting before any run.

And remove the `| tee` and egrep commands since we don't need this
workaround to show error.

REF:

1: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell

ISSUE: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 20, 2023
According to Github Actions doc [1], linux platform will use `bash -e
{0}` mode to run script. But we also need `pipefail` and `nounset` in
CI. This patch is aimed to add strict mode setting before any run.

And remove the `| tee` and egrep commands since we don't need this
workaround to show error.

REF:

1: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell

ISSUE: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 20, 2023
According to Github Actions doc [1], linux platform will use `bash -e
{0}` mode to run script. But we also need `pipefail` and `nounset` in
CI. This patch is aimed to add strict mode setting before any run.

And remove the `| tee` and egrep commands since we don't need this
workaround to show error.

REF:

1: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell

ISSUE: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 20, 2023
According to Github Actions doc [1], linux platform will use `bash -e
{0}` mode to run script. But we also need `pipefail` and `nounset` in
CI. This patch is aimed to add strict mode setting before any run.

And remove the `| tee` and egrep commands since we don't need this
workaround to show error.

REF:

1: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell

ISSUE: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 21, 2023
According to Github Actions doc [1], linux platform will use `bash -e
{0}` mode to run script. But we also need `pipefail` and `nounset` in
CI. This patch is aimed to add strict mode setting before any run.

And remove the `| tee` and egrep commands since we don't need this
workaround to show error.

REF:

1: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell

ISSUE: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 21, 2023
According to Github Actions doc [1], linux platform will use `bash -e
{0}` mode to run script. But we also need `pipefail` and `nounset` in
CI. This patch is aimed to add strict mode setting before any run.

And remove the `| tee` and egrep commands since we don't need this
workaround to show error.

REF:

1: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell

ISSUE: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 22, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 22, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Mar 22, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@serathius
Copy link
Member Author

@fuweid Is there anything left or can I close the issue?

@fuweid
Copy link
Member

fuweid commented Apr 8, 2023

@serathius I am working on ./tests/manual/docker-static-ip/* scripts. Will file pr later.

@serathius
Copy link
Member Author

@fuweid Those are legacy scripts that are no longer used. We should consider removing them.

fuweid added a commit to fuweid/etcd that referenced this issue Apr 9, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 9, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 9, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 9, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 11, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 11, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 11, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 11, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 13, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 13, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 13, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this issue Apr 13, 2023
REF: etcd-io#15514

Signed-off-by: Wei Fu <fuweid89@gmail.com>
ahrtr added a commit that referenced this issue Apr 13, 2023
chore: cleanup #15514 (Rewrite etcd scripts in strict mode)
@fuweid
Copy link
Member

fuweid commented Apr 13, 2023

The cleanup is done. I am closing it.

@fuweid fuweid closed this as completed Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants