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

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

Closed
sophia-guo opened this issue Dec 8, 2020 · 35 comments · Fixed by ibmruntimes/ci-jenkins-pipelines#108 or #5034
Assignees

Comments

@sophia-guo
Copy link
Contributor

sophia-guo commented Dec 8, 2020

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
Copy link

I would like to work on this issue.

@abigailsleek
Copy link

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
Copy link
Contributor Author

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
Copy link

abigailsleek commented Oct 16, 2021 via email

@abigailsleek
Copy link

abigailsleek commented Oct 16, 2021 via email

@llxia
Copy link
Contributor

llxia commented Oct 18, 2021

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
Copy link

abigailsleek commented Oct 18, 2021 via email

@sophia-guo
Copy link
Contributor Author

yes, the changes or suggestion in #2097 (comment) 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
Copy link

abigailsleek commented Oct 18, 2021 via email

@llxia
Copy link
Contributor

llxia commented Oct 18, 2021

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

@abigailsleek
Copy link

abigailsleek commented Oct 18, 2021 via email

@abigailsleek
Copy link

abigailsleek commented Oct 23, 2021 via email

@smlambert
Copy link
Contributor

smlambert commented Oct 23, 2021

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
Copy link

abigailsleek commented Oct 23, 2021 via email

@abigailsleek
Copy link

abigailsleek commented Oct 26, 2021 via email

@YeeSkywalker
Copy link
Contributor

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

@llxia
Copy link
Contributor

llxia commented Sep 14, 2022

Yes, it is a good first issue.

@julian55455
Copy link
Contributor

julian55455 commented Oct 13, 2022

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

@zdtsw
Copy link
Contributor

zdtsw commented Oct 13, 2022

@smlambert can you assign it to @julian55455 ?

@sophia-guo
Copy link
Contributor Author

@llxia Has the mismatch been done #2097 (comment) ?

@llxia
Copy link
Contributor

llxia commented Oct 13, 2022

No, it is a good first issue.

@sophia-guo
Copy link
Contributor Author

@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?

@llxia
Copy link
Contributor

llxia commented Oct 13, 2022

I do not see the issue tap in that repo as well. Maybe we can open the issue in aqa-tests instead.

@Tishly
Copy link

Tishly commented Oct 16, 2022

Hi @llxia, Is this issue still open? I would like to work on it.

@zdtsw
Copy link
Contributor

zdtsw commented Oct 17, 2022

Hi @llxia, Is this issue still open? I would like to work on it.

I think @julian55455 is on it already? I do not have permission on this repo to assign her, maybe @smlambert or @llxia can help

@llxia
Copy link
Contributor

llxia commented Oct 17, 2022

@julian55455 I assigned this issue to you. Thanks

@Ndacyayisenga-droid
Copy link
Contributor

@smlambert should this issue still be open?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment