Adapt PR and release jobs to enable testing of the Framework repo #170

bensze01 · 2024-07-03T16:57:48Z

This PR enables the testing of the framework repo, by adapting the existing jobs to allow overriding the version of the framework checked out by the CI, as well as enabling the testing of multiple branches of Mbed TLS in a single job.

The framework version is controlled by the newly defined parameters FRAMEWORK_REPO and FRAMEWORK_BRANCH, while MBEDTLS_BRANCH has been extended to allow specifying multiple branches separated by ,.

For automatic PR testing, mbedtls.run_framework_pr_job() is added as an entrypoint, which is called by the Jenkinsfile added by the companion PR Mbed-TLS/mbedtls-framework#33

Tests:

Runs with framework version override enabled:
Runs without a framework version override:
- development: PR-head, PR-merge
- mbedtls-3.6: PR-head, PR-merge
- mbedtls-2.28: PR-head, PR-merge
- mbed-tls-pr-test-parametrized
- mbedtls-release-ci-testing
- Supposed to fail:

minosgalanakis

Looks ok overall, with some minor comments.

To my understanding this change will make the ci do two checkous per branch? One for mbedtls and then the framework indipendantly?

Looking at the combinations of MBEDTLS_BRANCH , FRAMEWORK_BRANCH times the number of branches, that can cause a significant overhead.

Could we download a non shallow copy of MBEDTLS_REPO and share it between jobs, by simply changing the appropriate branch?

vars/environ.groovy

vars/analysis.groovy

Signed-off-by: Bence Szépkúti <[email protected]>

Pass the branch name along the call chain instead. This is in preparation for testing multiple branches in the same job. Signed-off-by: Bence Szépkúti <[email protected]>

Signed-off-by: Bence Szépkúti <[email protected]>

Each branch listed in MBED_TLS_BRANCH (separated by commas) will be checked out and tested in parallel. Signed-off-by: Bence Szépkúti <[email protected]>

Signed-off-by: Bence Szépkúti <[email protected]>

IllegalAccessError doesn't match the semantics used here. Signed-off-by: Bence Szépkúti <[email protected]>

This fixes checkouts for productions jobs that target the framework repo. Signed-off-by: Bence Szépkúti <[email protected]>

This commit also refactors failed_builds and outcome_stashes to be per-branch values by moving them inside BranchInfo. Signed-off-by: Bence Szépkúti <[email protected]>

Signed-off-by: Bence Szépkúti <[email protected]>

minosgalanakis

LGTM

minosgalanakis

LGTM

mpg · 2024-10-10T08:11:30Z

@bensze01 Are the labels needs-ci and needs-work still relevant?

bensze01 · 2024-10-10T11:47:23Z

@mpg Needs work is not, needs CI is.

Signed-off-by: Bence Szépkúti <[email protected]>

bensze01 · 2024-10-28T07:32:38Z

@minosgalanakis @mpg All the CI runs have passed successfully, please (re-)review!

minosgalanakis

LGTM

mpg

Review in progress; next dc9d984

mpg · 2024-11-04T09:14:27Z

src/org/mbed/tls/jenkins/BranchInfo.groovy

@@ -4,27 +4,27 @@ class BranchInfo {
    /** The name of the branch */
    public String branch

-    /* Map from component name to chosen platform to run it, or to null
-     * if no platform has been chosen yet. */
+    /** Map from component name to chosen platform to run it, or to null


Out of curiosity: are we running javadoc on this? If so, where's the output? If not how would we go about it?

mpg · 2024-11-04T09:57:31Z

vars/environ.groovy

@@ -45,6 +45,10 @@ void set_pr_environment(String target_repo, boolean is_production) {

 def set_common_pr_production_environment() {
    env.CHECKOUT_METHOD = 'scm'
+    /* The credentials here are the SSH credentials for accessing the repositories.
+       They are defined at {JENKINS_URL}/credentials
+       This is a temporary workaround, this should really be set in the Jenkins job configs */


Do we have plans to remove this temporary work-around then? Is it tracked somewhere?

mpg · 2024-11-04T10:29:01Z

vars/analysis.groovy

@@ -41,9 +41,6 @@ import net.sf.json.JSONObject
 import org.mbed.tls.jenkins.BranchInfo
 import org.mbed.tls.jenkins.JobTimestamps

-// A static field has its content preserved across stages.


Sorry if this should be obvious, but I'm not seeing how the info is preserved accross stages now that it's stored as part of BranchInfo.

(I can see from the test runs that it is preserved, I just can't understand how from looking at the code.)

mpg

LGTM, thanks! None of my questions are blockers, so I'm approving this.

bensze01 added needs: ci needs: review needs: reviewer priority-high labels Jul 3, 2024

bensze01 mentioned this pull request Jul 3, 2024

Add Jenkinsfile Mbed-TLS/mbedtls-framework#33

Merged

bensze01 force-pushed the dev/bensze01/framework branch 3 times, most recently from 169671d to 5c04cf3 Compare July 3, 2024 17:46

bensze01 added size-m Estimated task size: medium (~1w) needs: work and removed needs: work labels Jul 3, 2024

bensze01 force-pushed the dev/bensze01/framework branch from 60272b5 to 4eec09e Compare July 9, 2024 23:28

bensze01 added the needs: work label Jul 11, 2024

ronald-cron-arm requested a review from minosgalanakis July 23, 2024 09:40

ronald-cron-arm self-requested a review August 28, 2024 13:21

ronald-cron-arm removed the needs: reviewer label Aug 28, 2024

ronald-cron-arm removed their request for review September 18, 2024 13:10

minosgalanakis reviewed Sep 24, 2024

View reviewed changes

vars/environ.groovy Show resolved Hide resolved

vars/analysis.groovy Outdated Show resolved Hide resolved

bensze01 mentioned this pull request Oct 2, 2024

Fix analysis step to use only a single helper-container-host executor #177

Merged

bensze01 added 8 commits October 8, 2024 15:34

Remove unused variable REPO_TO_CHECKOUT

525f7fd

Signed-off-by: Bence Szépkúti <[email protected]>

Rename tls-specific methods in checkout_repo

7224715

Signed-off-by: Bence Szépkúti <[email protected]>

Remove deep references to global env.MBED_TLS_BRANCH

8f9814d

Pass the branch name along the call chain instead. This is in preparation for testing multiple branches in the same job. Signed-off-by: Bence Szépkúti <[email protected]>

JavaDoc-ify BranchInfo comments

86f2433

Signed-off-by: Bence Szépkúti <[email protected]>

Support testing multiple branches at once

a5bc97b

Each branch listed in MBED_TLS_BRANCH (separated by commas) will be checked out and tested in parallel. Signed-off-by: Bence Szépkúti <[email protected]>

Add framework testing jobs

c81b708

Signed-off-by: Bence Szépkúti <[email protected]>

Fix the exception type thrown by JobTimestamps

9495321

IllegalAccessError doesn't match the semantics used here. Signed-off-by: Bence Szépkúti <[email protected]>

Set GIT_CREDENTIALS_ID in production jobs

33efbb8

This fixes checkouts for productions jobs that target the framework repo. Signed-off-by: Bence Szépkúti <[email protected]>

bensze01 added 3 commits October 8, 2024 16:25

Handle multiple branches in analyze_results()

94574a3

This commit also refactors failed_builds and outcome_stashes to be per-branch values by moving them inside BranchInfo. Signed-off-by: Bence Szépkúti <[email protected]>

Refactor send_email to take a BranchInfo value

dc9d984

Signed-off-by: Bence Szépkúti <[email protected]>

Convert coverage_details into a per-branch value

8d9b2b7

Signed-off-by: Bence Szépkúti <[email protected]>

bensze01 force-pushed the dev/bensze01/framework branch from a06714d to fbd6e4c Compare October 9, 2024 13:38

minosgalanakis previously approved these changes Oct 9, 2024

View reviewed changes

bensze01 dismissed minosgalanakis’s stale review via 8d9b2b7 October 9, 2024 15:56

bensze01 force-pushed the dev/bensze01/framework branch from fbd6e4c to 8d9b2b7 Compare October 9, 2024 15:56

minosgalanakis previously approved these changes Oct 9, 2024

View reviewed changes

mpg self-requested a review October 10, 2024 08:13

bensze01 removed the needs: work label Oct 10, 2024

bensze01 added 2 commits October 25, 2024 16:35

Fix platform name

7caa948

Signed-off-by: Bence Szépkúti <[email protected]>

Only compress outcomes.csv if the file exists

bdc39b2

Signed-off-by: Bence Szépkúti <[email protected]>

bensze01 dismissed minosgalanakis’s stale review via bdc39b2 October 25, 2024 14:35

bensze01 requested a review from minosgalanakis October 28, 2024 07:31

bensze01 removed the needs: ci label Oct 28, 2024

minosgalanakis approved these changes Oct 28, 2024

View reviewed changes

mpg reviewed Nov 4, 2024

View reviewed changes

mpg approved these changes Nov 5, 2024

View reviewed changes

mpg added approved Approved in review. May need additional CI. and removed needs: review labels Nov 5, 2024

mpg merged commit b2f594f into main Nov 5, 2024
1 check passed

mpg mentioned this pull request Nov 6, 2024

Add CI support Mbed-TLS/mbedtls-framework#11

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt PR and release jobs to enable testing of the Framework repo #170

Adapt PR and release jobs to enable testing of the Framework repo #170

bensze01 commented Jul 3, 2024 •

edited

Loading

minosgalanakis left a comment

minosgalanakis left a comment

minosgalanakis left a comment

mpg commented Oct 10, 2024

bensze01 commented Oct 10, 2024

bensze01 commented Oct 28, 2024

minosgalanakis left a comment

mpg left a comment

mpg Nov 4, 2024

mpg Nov 4, 2024

mpg Nov 4, 2024

mpg Nov 4, 2024

mpg left a comment

Adapt PR and release jobs to enable testing of the Framework repo #170

Adapt PR and release jobs to enable testing of the Framework repo #170

Conversation

bensze01 commented Jul 3, 2024 • edited Loading

minosgalanakis left a comment

Choose a reason for hiding this comment

minosgalanakis left a comment

Choose a reason for hiding this comment

minosgalanakis left a comment

Choose a reason for hiding this comment

mpg commented Oct 10, 2024

bensze01 commented Oct 10, 2024

bensze01 commented Oct 28, 2024

minosgalanakis left a comment

Choose a reason for hiding this comment

mpg left a comment

Choose a reason for hiding this comment

mpg Nov 4, 2024

Choose a reason for hiding this comment

mpg Nov 4, 2024

Choose a reason for hiding this comment

mpg Nov 4, 2024

Choose a reason for hiding this comment

mpg Nov 4, 2024

Choose a reason for hiding this comment

mpg left a comment

Choose a reason for hiding this comment

bensze01 commented Jul 3, 2024 •

edited

Loading