Switching to multi-JDK image, enriching build manifest with desired JDK version #769

reta · 2021-10-20T14:43:19Z

Signed-off-by: Andriy Redko [email protected]

Description

Use new CI Docker images, enriching build manifest with desired JDK version. The build manifest has been enriched with new optional section ci:

ci:
  image:
     name: "opensearchstaging/ci-runner:centos7-x64-arm64-jdkmulti-node10.24.1-cypress6.9.1-20211019"
     args: "-e JAVA_HOME=/opt/java/openjdk-11"
build:
  name: OpenSearch
  version: 2.0.0

This build manifest is being read in preparation to running Jenkins job and the its JDK version, if specified, is enforced (the defaults stay unchanged and use JDK-14).

Issues Resolved

Part of #732

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

reta · 2021-10-20T16:25:43Z

@peterzhuamazon @dblock what do you think about this solution guys? Ideally, it would be great to pick the JDK right from the branch itself (.ci/java-versions.properties), but the checkout is buried deeply in the build scripts so it is not possible to get there from the Jenkinsfile, where only build manifests are accessible.

dblock

I think it's alright, but I wonder whether we can get closer to what GHA does?

ci:
  image:
     - multi-whatever-docker-container-thing
  env:
     - JAVA_HOME=...

Putting those in the manifest would make Jenkinsfile a lot lighter. The environment should be loaded and set when executing the build.

reta · 2021-10-20T17:08:16Z

@dblock I think your idea is good but would make maintaining / updating images challenging - it is in one place now, but we would have to update each and every manifest otherwise. Also, I think the manifests are high level in a sense they say what is needed but not how to get that: for example jdk: 11 says I need to build this branch using JDK-11, whereas env / JAVA_HOME just imposes the environment right away. WDYT?

dblock · 2021-10-20T18:21:39Z

@dblock I think your idea is good but would make maintaining / updating images challenging - it is in one place now, but we would have to update each and every manifest otherwise.

We should not be updating manifests for versions that have been released. Hence the manifest sort of needs to know which VM it was built against, or it might need more fixes to work on a newer VM when we decide to backport, for example, a security patch.

Also, I think the manifests are high level in a sense they say what is needed but not how to get that: for example jdk: 11 says I need to build this branch using JDK-11, whereas env / JAVA_HOME just imposes the environment right away. WDYT?

I understand and agree. That's why GHA came up with a concept of actions that abstract this completely. That's totally fine by me if we implement that in Python. I would just not add this logic into Jenkinsfile because it quickly becomes a huge mess. You could introduce a dependencies section into the manifest that could start with a simple list like jdk=14, get resolved to a python class and anything could be executed there, such as output an environment that needs to be set when executing the build scripts.

In general though, we're reinventing GHA and that concept and product has been exceptional IMO, we just weren't able to use it because reasons. I'd stick close to that.

reta · 2021-10-20T18:36:37Z

Oh, right, the manifests are for exact versions, agree with you, a bit lost which Jenkinsfile is for what, in this case we could certainly have images included in the manifest, @peterzhuamazon, any objection from your side?

I understand and agree. That's why GHA came up with a concept of actions that abstract this completely. That's totally fine by me if we implement that in Python.

I would +1 following GHA idioms. But I am completely lost in Python, it goes way beyond my expertise, @peterzhuamazon could you give me a hand here? I can definitely implement the image support in manifest but will drown in dependencies + build environment manipulation.

@dblock thank you, I was looking also into https://github.com/opensearch-project/OpenSearch/blob/main/Jenkinsfile and it seems like it uses predefined agents search-cloud-ec2-c518xlarge, which we have to rebuild out of new images, any hints / people who could help with that?

dblock · 2021-10-20T19:18:23Z

I would +1 following GHA idioms. But I am completely lost in Python, it goes way beyond my expertise, @peterzhuamazon could you give me a hand here? I can definitely implement the image support in manifest but will drown in dependencies + build environment manipulation.

I'd be happy to help. Try getting the image going and I can take care of dependences on top of that?

@dblock thank you, I was looking also into https://github.com/opensearch-project/OpenSearch/blob/main/Jenkinsfile and it seems like it uses predefined agents search-cloud-ec2-c518xlarge, which we have to rebuild out of new images, any hints / people who could help with that?

@peternied @peterzhuamazon ?

…DK version Signed-off-by: Andriy Redko <[email protected]>

dblock

I like this better!

Needs to propagate to OpenSearch Dashboards Jenkinsfile.

Add some rudimentary tests for the name and args in Python.

dblock · 2021-10-20T20:09:39Z

jenkins/opensearch/Jenkinsfile

+
+                    dockerImage = "${manifest.ci?.image?.name}" 
+                    // If the 'image' key is not present, it is populated with "null" string 
+                    if (dockerImage == null || dockerImage == "null") {


Let's hard fail here and require the image in CI?

Signed-off-by: Andriy Redko <[email protected]>

codecov-commenter · 2021-10-20T20:56:19Z

Codecov Report

Merging #769 (c33e363) into main (60345d6) will increase coverage by 0.00%.
The diff coverage is 91.66%.

@@           Coverage Diff           @@
##             main     #769   +/-   ##
=======================================
  Coverage   91.30%   91.30%           
=======================================
  Files          75       75           
  Lines        1990     2002   +12     
=======================================
+ Hits         1817     1828   +11     
- Misses        173      174    +1

Impacted Files	Coverage Δ
src/manifests/input_manifest.py	`95.91% <91.66%> (-1.38%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60345d6...c33e363. Read the comment docs.

Signed-off-by: Andriy Redko <[email protected]>

reta · 2021-10-20T21:39:43Z

@dblock seems like all your comments have been addressed with respect to new ci section :-)

manifests/1.2.0/opensearch-1.2.0.yml

peterzhuamazon · 2021-10-20T21:50:42Z

I would +1 following GHA idioms. But I am completely lost in Python, it goes way beyond my expertise, @peterzhuamazon could you give me a hand here? I can definitely implement the image support in manifest but will drown in dependencies + build environment manipulation.

I'd be happy to help. Try getting the image going and I can take care of dependences on top of that?

@dblock thank you, I was looking also into https://github.com/opensearch-project/OpenSearch/blob/main/Jenkinsfile and it seems like it uses predefined agents search-cloud-ec2-c518xlarge, which we have to rebuild out of new images, any hints / people who could help with that?

@peternied @peterzhuamazon ?

Are you trying to ask what change have I done to that Jenkins Agent?

dblock

👏 nice work @reta, you just learned Python :)

manifests/1.2.0/opensearch-1.2.0.yml

dblock · 2021-10-20T23:08:22Z

jenkins/opensearch-dashboards/Jenkinsfile

@@ -21,13 +21,36 @@ pipeline {
                }
            }
        }
+        stage('detect Docker image + args to use for the build') {


How do we reuse things in Jenkins 💭

One I know of is by using pipeline libraries

Someone will have to show us how :)

It sadly needs access to Jenkins instance and happens at job / pipeline level

reta · 2021-10-20T23:26:32Z

Are you trying to ask what change have I done to that Jenkins Agent?

@peterzhuamazon more precisely, if these agents use the same CI multi-jdk images you've published or different ones?

peterzhuamazon · 2021-10-21T14:45:21Z

Are you trying to ask what change have I done to that Jenkins Agent?

@peterzhuamazon more precisely, if these agents use the same CI multi-jdk images you've published or different ones?

They use the one you define in the Jenkinsfile.

reta · 2021-10-21T14:56:03Z

They use the one you define in the Jenkinsfile.

@peterzhuamazon Sorry, I am not following, this is a complete Jenkinsfile (https://github.com/opensearch-project/OpenSearch/blob/main/Jenkinsfile), it uses labelled agents which have to be provided by Jenkins.

pipeline {
    agent { label 'search-cloud-ec2-c518xlarge' }


    stages {
        stage('Build') {
            steps {
                echo 'Building..'
                // Disable backward compability tasks
                sh './gradlew check --no-daemon --no-scan -Pbwc_tests_enabled=false'
            }
        }
        stage('Test') {
            steps {
                echo 'Testing..'
            }
        }
        stage('Deploy') {
            steps {
                echo 'Deploying..'
            }
        }
    }
}

peterzhuamazon · 2021-10-21T16:00:36Z

They use the one you define in the Jenkinsfile.

@peterzhuamazon Sorry, I am not following, this is a complete Jenkinsfile (https://github.com/opensearch-project/OpenSearch/blob/main/Jenkinsfile), it uses labelled agents which have to be provided by Jenkins.
pipeline {
    agent { label 'search-cloud-ec2-c518xlarge' }


    stages {
        stage('Build') {
            steps {
                echo 'Building..'
                // Disable backward compability tasks
                sh './gradlew check --no-daemon --no-scan -Pbwc_tests_enabled=false'
            }
        }
        stage('Test') {
            steps {
                echo 'Testing..'
            }
        }
        stage('Deploy') {
            steps {
                echo 'Deploying..'
            }
        }
    }
}

The docker image will run in those agent you defined.
No need to change the agent as long as you define the docker image, they will for sure run in those Jenkins agent.
The Jenkins agent act as a Docker Host.

https://github.com/opensearch-project/opensearch-build/blob/main/jenkins/opensearch-dashboards/Jenkinsfile#L27-L32

Edit: ignore my comments I was not getting what @reta means earlier.

peterzhuamazon · 2021-10-21T16:02:29Z

Oh I see what you are talking about @reta I dont think you need to change the OpenSearch Jenkinsfile as that one has not been used. That is a very early Jenkinsfile for testing.

@dblock we probably should remove that at this point to avoid confusion.

reta · 2021-10-21T16:12:43Z

@peterzhuamazon thank you, to completely erase the confusion, what Jenkinsfile is being used to build main (== 2.0)? Not like I see any manifests in there https://github.com/opensearch-project/opensearch-build/blob/main/jenkins/opensearch/Jenkinsfile#L5 ...

peterzhuamazon · 2021-10-21T16:28:07Z

@peterzhuamazon thank you, to completely erase the confusion, what Jenkinsfile is being used to build main (== 2.0)? Not like I see any manifests in there https://github.com/opensearch-project/opensearch-build/blob/main/jenkins/opensearch/Jenkinsfile#L5 ...

We are using freestyle in Jenkins to build dashboards min, which we still need to convert to pipeline Jenkinsfile.
We are using the exact Jenkinsfile in this PR to build opensearch min and bundle.

Thanks.

dblock · 2021-10-21T17:26:04Z

@peterzhuamazon Is this good to merge? Needs your approval.

peterzhuamazon

Thanks.

reta force-pushed the new.image branch 6 times, most recently from 942ea57 to 4632a62 Compare October 20, 2021 16:05

dblock requested changes Oct 20, 2021

View reviewed changes

Switching to multi-JDK image, enriching build manifest with desired J…

fd024c9

…DK version Signed-off-by: Andriy Redko <[email protected]>

reta force-pushed the new.image branch 3 times, most recently from ad0863d to bdcedc2 Compare October 20, 2021 20:03

dblock reviewed Oct 20, 2021

View reviewed changes

Refactoring manifest to include CI section with image and args

9bedf46

Signed-off-by: Andriy Redko <[email protected]>

reta force-pushed the new.image branch 2 times, most recently from 84ffba6 to 13de88b Compare October 20, 2021 20:46

Added test cases

c33e363

Signed-off-by: Andriy Redko <[email protected]>

reta force-pushed the new.image branch from 13de88b to c33e363 Compare October 20, 2021 20:53

Migrated opensearch-dashboards to use CI image from the build manifest

92de567

Signed-off-by: Andriy Redko <[email protected]>

reta force-pushed the new.image branch from 3c5069b to 92de567 Compare October 20, 2021 21:18

peterzhuamazon reviewed Oct 20, 2021

View reviewed changes

manifests/1.2.0/opensearch-1.2.0.yml Show resolved Hide resolved

dblock approved these changes Oct 20, 2021

View reviewed changes

manifests/1.2.0/opensearch-1.2.0.yml Show resolved Hide resolved

dblock reviewed Oct 20, 2021

View reviewed changes

dblock requested a review from peternied October 20, 2021 23:08

reta mentioned this pull request Oct 21, 2021

Removing Jenkinsfile (not used), replaced by opensearch-build/jenkins/opensearch/Jenkinsfile opensearch-project/OpenSearch#1408

Merged

5 tasks

dblock requested a review from peterzhuamazon October 21, 2021 17:25

peterzhuamazon approved these changes Oct 21, 2021

View reviewed changes

peterzhuamazon merged commit 626255c into opensearch-project:main Oct 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switching to multi-JDK image, enriching build manifest with desired JDK version #769

Switching to multi-JDK image, enriching build manifest with desired JDK version #769

reta commented Oct 20, 2021 •

edited

Loading

reta commented Oct 20, 2021 •

edited

Loading

dblock left a comment

reta commented Oct 20, 2021

dblock commented Oct 20, 2021 •

edited

Loading

reta commented Oct 20, 2021 •

edited

Loading

dblock commented Oct 20, 2021

dblock left a comment

dblock Oct 20, 2021

reta Oct 20, 2021

codecov-commenter commented Oct 20, 2021 •

edited

Loading

reta commented Oct 20, 2021

peterzhuamazon commented Oct 20, 2021

dblock left a comment

dblock Oct 20, 2021

reta Oct 20, 2021

dblock Oct 21, 2021

reta Oct 21, 2021

reta commented Oct 20, 2021 •

edited

Loading

peterzhuamazon commented Oct 21, 2021

reta commented Oct 21, 2021

peterzhuamazon commented Oct 21, 2021 •

edited

Loading

peterzhuamazon commented Oct 21, 2021 •

edited

Loading

reta commented Oct 21, 2021

peterzhuamazon commented Oct 21, 2021

dblock commented Oct 21, 2021

peterzhuamazon left a comment

Switching to multi-JDK image, enriching build manifest with desired JDK version #769

Switching to multi-JDK image, enriching build manifest with desired JDK version #769

Conversation

reta commented Oct 20, 2021 • edited Loading

Description

Issues Resolved

Check List

reta commented Oct 20, 2021 • edited Loading

dblock left a comment

Choose a reason for hiding this comment

reta commented Oct 20, 2021

dblock commented Oct 20, 2021 • edited Loading

reta commented Oct 20, 2021 • edited Loading

dblock commented Oct 20, 2021

dblock left a comment

Choose a reason for hiding this comment

dblock Oct 20, 2021

Choose a reason for hiding this comment

reta Oct 20, 2021

Choose a reason for hiding this comment

codecov-commenter commented Oct 20, 2021 • edited Loading

Codecov Report

reta commented Oct 20, 2021

peterzhuamazon commented Oct 20, 2021

dblock left a comment

Choose a reason for hiding this comment

dblock Oct 20, 2021

Choose a reason for hiding this comment

reta Oct 20, 2021

Choose a reason for hiding this comment

dblock Oct 21, 2021

Choose a reason for hiding this comment

reta Oct 21, 2021

Choose a reason for hiding this comment

reta commented Oct 20, 2021 • edited Loading

peterzhuamazon commented Oct 21, 2021

reta commented Oct 21, 2021

peterzhuamazon commented Oct 21, 2021 • edited Loading

peterzhuamazon commented Oct 21, 2021 • edited Loading

reta commented Oct 21, 2021

peterzhuamazon commented Oct 21, 2021

dblock commented Oct 21, 2021

peterzhuamazon left a comment

Choose a reason for hiding this comment

reta commented Oct 20, 2021 •

edited

Loading

reta commented Oct 20, 2021 •

edited

Loading

dblock commented Oct 20, 2021 •

edited

Loading

reta commented Oct 20, 2021 •

edited

Loading

codecov-commenter commented Oct 20, 2021 •

edited

Loading

reta commented Oct 20, 2021 •

edited

Loading

peterzhuamazon commented Oct 21, 2021 •

edited

Loading

peterzhuamazon commented Oct 21, 2021 •

edited

Loading