Skip to content

Conversation

@shirady
Copy link
Contributor

@shirady shirady commented Jan 23, 2023

Signed-off-by: shirady 57721533+shirady@users.noreply.github.com

Explain the changes

According to CI next steps meeting:

  1. Change default timeouts for our tests to 1.5h (90 minutes).
  2. Change all checkout to v3 (v2 is using node version 12).
  3. Use GH Concurrency to cancel the previous run.
    This was a pilot in Add a step to cancel previous runs in GH Action #7159 , but after we started using it, we discovered that there is a builtin option from GitHub (concurrency).

General cleanup (was not in the meeting):

  1. Remove exit 1 since the default behavior is when a step is failing to fail the whole job.
  2. Remove extra newlines.
  3. Remove comments that explain the code itself.
  4. Remove unused id fields in the steps.

Issues: Gap

The main gap is that the default timeout of jobs is 6 hours and it is a waste of computing resources since all of our jobs run in a sixth of the time. Another waste of computing resources is when we use successive pushes, and we would like to reduce it.

Testing Instructions:

  1. none
  • Doc added/updated
  • Tests added

Copy link
Contributor

@liranmauda liranmauda left a comment

Choose a reason for hiding this comment

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

@shirady The reason that we added || exit 1 was that sometimes steps failed and the tests were green. In this case, we will not be aware that the test failed as we are not looking into green tests (which makes sense).
If we want to remove the || exit 1 we need to be sure that failing steps will lead to red tests (failing GH Action tests).

@shirady
Copy link
Contributor Author

shirady commented Jan 23, 2023

@shirady The reason that we added || exit 1 was that sometimes steps failed and the tests were green. In this case, we will not be aware that the test failed as we are not looking into green tests (which makes sense). If we want to remove the || exit 1 we need to be sure that failing steps will lead to red tests (failing GH Action tests).

@liranmauda,
From previous trial and error, we know that if a step fails then all job will fail.
Anyway, I did a test: I add an exit 1 to the file run_npm_test_on_test_container.sh and the job failed.
Note: it's a copy of noobaa-core, this link will not be available in the future.

@shirady shirady changed the title Changes to all GitHub workflows according to CI/CD meeting Changes to all GitHub workflows - timeout, checkout v3, concurrency, general cleanup Jan 24, 2023
…general cleanup

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
Copy link
Contributor

@liranmauda liranmauda left a comment

Choose a reason for hiding this comment

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

LGTM

@shirady shirady merged commit ed921c5 into noobaa:master Jan 25, 2023
@shirady shirady deleted the changes-to-all-actions branch January 29, 2023 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants