Setting CUSTOMIZED_SDK_URL and SDK_RESOURCE as non-customized ignores the SDK_RESOURCE value

[sophia-guo]

In the grinder setting it says Customized SDK URL, only set when SDK_RESOURCE customized. However if the user accidentally set both CUSTOMIZED_SDK_URL and SDK_RESOURCE ( non-customized) the system will take CUSTOMIZED_SDK_URL and ignore the SDK_RESOURCE's setting.

Senario:

1. First use CUSTOMIZED_SDK_URL
2. Rerun use SDK_RESOURCE = nightly to compare the difference ( CUSTOMIZED_SDK_URL is not reset as empty)
3. Build will rerun with CUSTOMIZED_SDK_URL, which ignores SDK_RESOURCE as nightly ( also in get.sh, SDK_RESOURCE default is nightly).

Solution: Enhance the check condition(https://github.com/adoptium/aqa-tests/blob/master/get.sh#L180) withif "$SDK_RESOURCE" == "customized" in two places.

1. https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L371-L373
2. https://github.com/adoptium/aqa-tests/blob/master/get.sh#L180

example： https://ci.adoptopenjdk.net/view/work-in-progress/job/grinder_sandbox/162/console

[abigailsleek]

I would like to work on this issue.

[abigailsleek]

A follow-up question for this issue. Please, I just to confirm that I understand what I am to do. I am to update this line https://github.com/adoptium/aqa-tests/blob/master/get.sh#L180 with if "$SDK_RESOURCE" == "customized" then this latestBuildUrl="${CUSTOMIZED_SDK_URL}${max}/" will follow. While for https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L371-L373 I just update with the if "$SDK_RESOURCE" == "customized" statement only. Yes?

[sophia-guo]

The logic is a little bit different as now we also allow users to provide a base artifactory URL (URL before the build number). For this issue we can update jenkinsfileBase only. No need to update get.sh anymore.

For jenkinsfileBase the Line360-L365 https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L360-L365 should be updated.

Customized SDK URL only set when SDK_RESOURCE is customized. That is as long as SDK_RESOURCE != "customized" ( except (params.SDK_RESOURCE == 'nightly' && params.CUSTOMIZED_SDK_URL)) we'd like to set CUSTOMIZED_SDK_URL_OPTION = "".

if (params.SDK_RESOURCE == 'nightly' && params.CUSTOMIZED_SDK_URL) {
	CUSTOMIZED_SDK_URL_OPTION = "-c '${params.CUSTOMIZED_SDK_URL}'"
} else if (params.SDK_RESOURCE != 'customized') {
	CUSTOMIZED_SDK_URL_OPTION = ""
} else if (params.CUSTOMIZED_SDK_URL) {
	CUSTOMIZED_SDK_URL_OPTION = "-c '${params.CUSTOMIZED_SDK_URL}'"
} else {
         error("For customized sdk resource, please provide the sdk url as CUSTOMIZED_SDK_URL")
}

And then the check for !params.CUSTOMIZED_SDK_URL in https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L373 can also be removed.

[abigailsleek]

Thank you for the clarification.

On Fri, 15 Oct 2021 at 9:41 PM, sophia-guo @.***> wrote:

The logic is a little bit different as now we also allow users to provide a base artifactory URL (URL before the build number). For this issue we can update jenkinsfileBase only. No need to update get.sh anymore.

For jenkinsfileBase the Line360-L365 https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L360-L365 should be updated.

Customized SDK URL only set when SDK_RESOURCE is customized. That is as long as SDK_RESOURCE != "customized" ( except (params.SDK_RESOURCE == 'nightly' && params.CUSTOMIZED_SDK_URL)) we'd like to set CUSTOMIZED_SDK_URL_OPTION = "".

if (params.SDK_RESOURCE == 'nightly' && params.CUSTOMIZED_SDK_URL) { CUSTOMIZED_SDK_URL_OPTION = "-c '${params.CUSTOMIZED_SDK_URL}'" } else if (params.SDK_RESOURCE != 'customized') { CUSTOMIZED_SDK_URL_OPTION = "" } else if (params.CUSTOMIZED_SDK_URL) { CUSTOMIZED_SDK_URL_OPTION = "-c '${params.CUSTOMIZED_SDK_URL}'" } else { error("For customized sdk resource, please provide the sdk url as CUSTOMIZED_SDK_URL") }

And then the check for !params.CUSTOMIZED_SDK_URL in https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L373 can also be removed.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-944647411, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITNLKGBD5BPIFAS4AOTUHCGZHANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

[abigailsleek]

Hello Sophia,

Please kindly review my pull request on this issue, and share with me your feedback.

Thank you.

On Sat, 16 Oct 2021 at 9:20 AM, Nneamaka Chalokwu @.***> wrote:

Thank you for the clarification.

On Fri, 15 Oct 2021 at 9:41 PM, sophia-guo @.***> wrote:

The logic is a little bit different as now we also allow users to provide a base artifactory URL (URL before the build number). For this issue we can update jenkinsfileBase only. No need to update get.sh anymore.

For jenkinsfileBase the Line360-L365 https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L360-L365 should be updated.

Customized SDK URL only set when SDK_RESOURCE is customized. That is as long as SDK_RESOURCE != "customized" ( except (params.SDK_RESOURCE == 'nightly' && params.CUSTOMIZED_SDK_URL)) we'd like to set CUSTOMIZED_SDK_URL_OPTION = "".

if (params.SDK_RESOURCE == 'nightly' && params.CUSTOMIZED_SDK_URL) { CUSTOMIZED_SDK_URL_OPTION = "-c '${params.CUSTOMIZED_SDK_URL}'" } else if (params.SDK_RESOURCE != 'customized') { CUSTOMIZED_SDK_URL_OPTION = "" } else if (params.CUSTOMIZED_SDK_URL) { CUSTOMIZED_SDK_URL_OPTION = "-c '${params.CUSTOMIZED_SDK_URL}'" } else { error("For customized sdk resource, please provide the sdk url as CUSTOMIZED_SDK_URL") }

And then the check for !params.CUSTOMIZED_SDK_URL in https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L373 can also be removed.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-944647411, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITNLKGBD5BPIFAS4AOTUHCGZHANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

-- Nneamaka Chalokwu

[llxia]

Please note that our scenario has changed. We also have the following case where: SDK_RESOURCE=nightly CUSTOMIZED_SDK_URL=<base URL from artifactory> In this case, our script will try to find the latest URL link.

[abigailsleek]

Okay, thank you. Does this mean the code needs to be updated? If so, where exactly.

On Mon, 18 Oct 2021 at 2:09 PM, Lan Xia @.***> wrote:

Please note that our scenario has changed. We also have the following case where: SDK_RESOURCE=nightly CUSTOMIZED_SDK_URL=<base URL from artifactory> In this case, our script will try to find the latest URL link.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-945750224, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITKSEEVBTHA2TEVW3RDUHQMCFANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

[sophia-guo]

yes, the changes or suggestion in https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-944647411 has included the scenario SDK_RESOURCE=nightly CUSTOMIZED_SDK_URL=<base URL from artifactory>

@abigailsleek I didn't find your changes, did you create a PR?

[abigailsleek]

Okay, thank you.

Yes I already created a pull request for this repository because I made changes to the testJobTemplate file. I’ll share the link to my commit on this issue so you can review it.

https://github.com/adoptium/aqa-tests/pull/2973/commits/3af6c7e71a06c3bc3b1bab0355b54f28663c5083

On Mon, 18 Oct 2021 at 3:37 PM, sophia-guo @.***> wrote:

yes, the changes or suggestion in #2097 (comment) https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-944647411 has included the scenario SDK_RESOURCE=nightly CUSTOMIZED_SDK_URL=<base URL from artifactory>

@abigailsleek https://github.com/abigailsleek I didn't find your changes, did you create a PR?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-945843684, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITI76ON23BXJGAWH3S3UHQWKFANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

[llxia]

Thanks @abigailsleek . Could you please create one PR per issue?

[abigailsleek]

Sure, I will do that.

On Mon, 18 Oct 2021 at 4:49 PM, Lan Xia @.***> wrote:

Thanks @abigailsleek https://github.com/abigailsleek . Could you please create one PR per issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-945915194, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITOBIGUY4VGUNC47VVLUHQ6YNANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

[abigailsleek]

Hi, I tried creating another PR but it didn't work because it’s on the same branch. Should I create another branch or I should pull request on one of the other branches?

On Mon, 18 Oct 2021 at 4:49 PM, Lan Xia @.***> wrote:

Thanks @abigailsleek https://github.com/abigailsleek . Could you please create one PR per issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-945915194, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITOBIGUY4VGUNC47VVLUHQ6YNANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

[smlambert]

I typically use 1 branch per issue (where each issue gets is separate PR to resolve it). That way, I can start each branch freshly synched with the main branch and it contains no other clutter or content from other PRs. (see 3 minute recording describing this practice for guidance).

[abigailsleek]

Thank you for this information.

On Sat, 23 Oct 2021 at 4:25 PM, Shelley Lambert @.***> wrote:

I typically use 1 branch per issue (where each issue gets is separate PR to resolve it). That way, I can start each branch freshly synched with the main branch and it contains no other clutter. (see 3 minute recording https://drive.google.com/file/d/1yTcXaA8vHgHMkkuDONKeDCEufgZJO-TJ/view?usp=sharing describing this practice for guidance).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-950168257, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITPPEDHU5SDNWTTIUE3UILHW3ANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

[abigailsleek]

Hi Sophia, here is the link to my pull request. Kindly review. Thank you.

https://github.com/adoptium/aqa-tests/pull/3099

On Sat, 23 Oct 2021 at 5:50 PM, Nneamaka Chalokwu @.***> wrote:

Thank you for this information.

On Sat, 23 Oct 2021 at 4:25 PM, Shelley Lambert @.***> wrote:

I typically use 1 branch per issue (where each issue gets is separate PR to resolve it). That way, I can start each branch freshly synched with the main branch and it contains no other clutter. (see 3 minute recording https://drive.google.com/file/d/1yTcXaA8vHgHMkkuDONKeDCEufgZJO-TJ/view?usp=sharing describing this practice for guidance).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-950168257, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMKITPPEDHU5SDNWTTIUE3UILHW3ANCNFSM4USF7ACA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Nneamaka Chalokwu

-- Nneamaka Chalokwu

[YeeSkywalker]

Hi there, I am wondering if I could take this one? It would be much appreciated!

[smlambert]

@llxia - the PR for this was reverted. Is the intention to keep this open and try again, or was the resolution that we could not proceed with this issue at all?

[llxia]

We need to update the upstream build pipeline to set correct SDK_RESOURCE values before this issue can be fixed.

• in Adoptium pipeline: https://github.com/adoptium/ci-jenkins-pipelines/blob/cee0b690b2a3a199b1f3f77892b6a1e18a20ea3b/pipelines/build/common/openjdk_build_pipeline.groovy#L295

• in openj9 pipeline: https://github.com/eclipse-openj9/openj9/blob/68f2d9d26b7918a718f160b9d60ac39bdbd80b38/buildenv/jenkins/common/pipeline-functions.groovy#L248 https://github.com/eclipse-openj9/openj9/blob/68f2d9d26b7918a718f160b9d60ac39bdbd80b38/buildenv/jenkins/common/pipeline-functions.groovy#L252

I will open issue in the upstream build pipeline repos.

[llxia]

Issue opened: https://github.com/adoptium/ci-jenkins-pipelines/issues/256 https://github.com/eclipse-openj9/openj9/issues/14616

[smlambert]

Both issues are now addressed (via https://github.com/adoptium/ci-jenkins-pipelines/pull/265 and https://github.com/eclipse-openj9/openj9/pull/14736). Shall we try to reapply the PR for this?

[llxia]

From what I can see, https://github.com/ibmruntimes/ci-jenkins-pipelines still have a mismatch and it needs to be updated. https://github.com/ibmruntimes/ci-jenkins-pipelines/blob/ibm/pipelines/build/common/openjdk_build_pipeline.groovy#L302 https://github.com/ibmruntimes/ci-jenkins-pipelines/blob/ibm/pipelines/build/common/openjdk_build_pipeline.groovy#L429

[sophia-guo]

@llxia What's the mismatch? Did you mean smoketest set sdk_resource as 'upstream' and runAqaTest doesn't explicitly setup sdk_resource?

Can this be fixed by explicitly setting sdk_resource as customized for runAqaTest around https://github.com/ibmruntimes/ci-jenkins-pipelines/blob/ibm/pipelines/build/common/openjdk_build_pipeline.groovy#L443

context.string(name: 'SDK_RESOURCE', value: "customized"),

[llxia]

Yes, it can and should be fixed. We just did not get a chance to address it.

[sophia-guo]

@llxia can someone work on this https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-1148716147? Probably as a good first issue?

[llxia]

Yes, it is a good first issue.

[julian55455]

@zdtsw Can I take up this as we wait for feedback on this issue

[zdtsw]

@smlambert can you assign it to @julian55455 ?

[sophia-guo]

@llxia Has the mismatch been done https://github.com/adoptium/aqa-tests/issues/2097#issuecomment-1148716147 ?

[llxia]

No, it is a good first issue.

[sophia-guo]

@llxia , it's a good first issue under https://github.com/ibmruntimes/ci-jenkins-pipelines? Seems I can't even have the permission to see issues. Is it open to all people or only to ibmers?