Skip to content

Fail if server cannot start when populating SCC#522

Merged
leochr merged 1 commit into
WASdev:mainfrom
ymanton:fix-populate-scc
Jul 21, 2023
Merged

Fail if server cannot start when populating SCC#522
leochr merged 1 commit into
WASdev:mainfrom
ymanton:fix-populate-scc

Conversation

@ymanton

@ymanton ymanton commented Jul 20, 2023

Copy link
Copy Markdown
Contributor

If the server fails to start when populating the SCC we should gracefully fail the build. Because the populate_scc.sh script sets the -e shell option, the shell will exit if any command returns non-zero, except in certain cases.

The populate_scc.sh script starts and stops the server in a compound expression using &&, which is one such case where a non-zero return from server start will be ignored.

Executing server start and server stop separately fixes this and has the desired behaviour; if the server fails to start execution will stop and the build will fail, rather than continuing on and leading to undefined behaviour.

If the server fails to start when populating the SCC we should
gracefully fail the build. Because the populate_scc.sh script sets
the -e shell option, the shell will exit if any command returns
non-zero, except in certain cases.

The populate_scc.sh script starts and stops the server in a
compound expression using &&, which is one such case
where a non-zero return from `server start` will be ignored.

Executing `server start` and `server stop` separately fixes this
and has the desired behaviour; if the server fails to start execution
will stop and the build will fail, rather than continuing on and
leading to undefined behaviour.

Signed-off-by: Younes Manton <ymanton@ca.ibm.com>
@ymanton

ymanton commented Jul 20, 2023

Copy link
Copy Markdown
Contributor Author

FYI @leochr.

Corresponding OL PR: OpenLiberty/ci.docker#428

@leochr leochr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ymanton Thanks for the PR. Looks good.

@leochr leochr merged commit 172e14e into WASdev:main Jul 21, 2023
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.

2 participants