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

Migrate CircleCI workflows to GitHub Actions (2/3) #37

Closed
wants to merge 3 commits into from

Conversation

AzfaarQureshi
Copy link

@AzfaarQureshi AzfaarQureshi commented Dec 10, 2020

Which problem is this solving?

As part of issue, this pull request migrates the loadtest, correctness and build-package jobs to GitHub Actions. windows-msi and the publish jobs will be migrated in PR 3/3

Migration Plan

We suggest having CircleCI and GitHub Action jobs run in parallel for a few weeks. After the GitHub Actions jobs are running fine for a week or so and then remove the CircleCI workflows from config.yml
cc- @alolita, @shovnik

@AzfaarQureshi AzfaarQureshi force-pushed the migrating-to-gha-2 branch 2 times, most recently from cbb2dd3 to 8b1029b Compare December 15, 2020 22:55
@AzfaarQureshi AzfaarQureshi force-pushed the migrating-to-gha-2 branch 2 times, most recently from 247c5f7 to 5f89284 Compare December 16, 2020 18:21
@AzfaarQureshi AzfaarQureshi changed the title Migrating to gha 2 Migrate CircleCI workflows to GitHub Actions (2/3) Dec 16, 2020
@AzfaarQureshi AzfaarQureshi self-assigned this Dec 16, 2020
@AzfaarQureshi AzfaarQureshi changed the base branch from master to migrating-to-gha December 16, 2020 18:24
Comment on lines +135 to +139
loadtestpre:
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.splitloadtest.outputs.matrix }}
steps:
Copy link
Author

@AzfaarQureshi AzfaarQureshi Dec 16, 2020

Choose a reason for hiding this comment

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

loadtestpre does some pre-processing so we can simulate CircleCI testsharding on GitHub Actions. This cuts down the runtime of loadtests from 15mins to ~5mins
This job does the following:

  1. Get a list of all loadtest tests using make testbed-list-loadtest
  2. Split tests into groups of two (TestIdleMode|TestBallastMemory, TestLog10kDPS|TestMetric10kDPS etc)
  3. Create a JSON containing each test grouping. This JSON will be consumed by the strategy.matrix of the loadtest job using fromJSON(). The strategy.matrix is how we get to run the loadtests in parallel

Comment on lines +160 to +177
TESTS="$(make -s testbed-list-loadtest | xargs echo|sed 's/ /|/g')"
TESTS=(${TESTS//|/ })
MATRIX="{\"include\":["
curr=""
for i in "${!TESTS[@]}"; do
if (( i > 0 && i % 2 == 0 )); then
curr+="|${TESTS[$i]}"
else
if [ -n "$curr" ] && (( i>1 )); then
MATRIX+=",{\"test\":\"$curr\"}"
elif [ -n "$curr" ]; then
MATRIX+="{\"test\":\"$curr\"}"
fi
curr="${TESTS[$i]}"
fi
done
MATRIX+="]}"
echo "::set-output name=matrix::$MATRIX"
Copy link
Author

@AzfaarQureshi AzfaarQureshi Dec 16, 2020

Choose a reason for hiding this comment

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

This section of the code is what creates the strategy.matrix JSON

Comment on lines +223 to +226
- name: Create test result archive # some test results have invalid characters
if: ${{ failure() || success() }}
continue-on-error: true
run: tar -cvf test_results.tar testbed/tests/results
Copy link
Author

Choose a reason for hiding this comment

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

  • making a tarball of the test results because some tests have filenames with invalid characters (namely the 0*0bytes test) which causes the upload action to fail.

  • if: ${{ failure() || success() }} this ensures that test results are uploaded even if the loadtest fails which can be useful for debugging

  • continue-on-error: true Sometimes (rarely, but still) the upload action fails due to a network error on github's side. This makes sure the entire job doesnt fail if the test results fail to upload

@@ -60,7 +60,7 @@ type DataReceiverBase struct {
Port int
}

const DefaultHost = "localhost"
const DefaultHost = "127.0.0.1"
Copy link
Author

Choose a reason for hiding this comment

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

this line was causing an error with the fluentbit log test since localhost could not be resolved on the github actions runners.

@@ -42,7 +42,7 @@ func TestIdleMode(t *testing.T) {
)
defer tc.Stop()

tc.SetResourceLimits(testbed.ResourceSpec{ExpectedMaxCPU: 4, ExpectedMaxRAM: 50})
tc.SetResourceLimits(testbed.ResourceSpec{ExpectedMaxCPU: 4, ExpectedMaxRAM: 55})
Copy link
Author

Choose a reason for hiding this comment

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

In the GitHub Action runners the RAM utilization is consistently higher (probably due to some other background processes on the GitHub machine). Loadtests don't pass unless these values are adjusted a little 😞

Copy link

Choose a reason for hiding this comment

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

Is this increase enough?

@shovnik shovnik self-requested a review December 16, 2020 20:43
Copy link

@alolita alolita left a comment

Choose a reason for hiding this comment

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

lgtm

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.

3 participants