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

Feat: Ensure that we have Windows images for both nanoserver and servercore, on Adoptium's JDK8,JDK11 and JDK17 [INFRA-3047] #202

Merged
merged 3 commits into from
Oct 13, 2021

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Oct 7, 2021

This PR introduces the following changes:

  • Ensure that all the existing Windows images are using Adoptium (Eclipse Temurin)

  • Add the new JDK17 Windows images

  • Ensure that each Windows images exists for all JDKs (8, 11 and 17) with a nanoserver and a windows server core each

  • Improve the Windows Build step's performance by using docker-compose (and a build-windows.yaml docker-compose file) for parallel building the Windows images

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!

  • Ensure that the pull request title represents the desired changelog entry

  • Please describe what you did
    - [ ] Link to relevant issues in GitHub or Jira
    - [ ] Link to relevant pull requests, esp. upstream and downstream changes
    - [ ] Ensure you have provided tests - that demonstrates feature works or fixes the issue

@dduportal dduportal requested a review from a team as a code owner October 7, 2021 14:59
@dduportal dduportal changed the title Feat: Ensure that we have Windows images for both nanoserver and servercore, on Adoptium's JDK8,JDK11 and JDK11 Feat: Ensure that we have Windows images for both nanoserver and servercore, on Adoptium's JDK8,JDK11 and JDK11 [INFRA-3047] Oct 7, 2021
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

if they work and don't add too long to the build time.

@dduportal
Copy link
Contributor Author

One of the modified images is failing to build. I'll check later.

@dduportal
Copy link
Contributor Author

A few discoveries while working on this:

  • All the nanoserver images can be derived from the windowsservercore easily (that would avoid the slow download part where we re-download the same git, git-lfs, remoting.jar...). That could be a nice improvement to ensure the windows images does not diverge over time
  • I'm trying to use docker-compose to have a centralized manifest for the windows images (wip). It would also greatly accelerate the build time since docker-compose build accepts a --parallel flag :)
  • I need to work on tests

@dduportal dduportal force-pushed the feat/full-adoptium branch 2 times, most recently from 97064d8 to ca50764 Compare October 12, 2021 07:25
@garethjevans garethjevans changed the title Feat: Ensure that we have Windows images for both nanoserver and servercore, on Adoptium's JDK8,JDK11 and JDK11 [INFRA-3047] Feat: Ensure that we have Windows images for both nanoserver and servercore, on Adoptium's JDK8,JDK11 and JDK17 [INFRA-3047] Oct 12, 2021
…d JDK8 are using Adoptimu (Eclipse Temurin) and are using the same steps

Signed-off-by: Damien Duportal <[email protected]>
@dduportal
Copy link
Contributor Author

@timja @MarkEWaite So this PR is ready to be merged.
It should improve the build time, even by adding images:

= Last Master Branch Build (no Docker caching, only focusing on the Windows Parallel Stage)

Ref. https://ci.jenkins.io/blue/organizations/jenkins/Packaging%2Fdocker-agent/detail/master/140/pipeline/9

- Total Windows time is 31min 07s
  - The build step (sequential docker build): 20:15:43 -> 20:34:41 => 18 min 58
  - The test step (sequential test harnesses): 20:34:41 -> 20:46:32 => 11 min 51

== This PR

https://ci.jenkins.io/blue/organizations/jenkins/Packaging%2Fdocker-agent/detail/PR-202/37/pipeline/9 (first success and complete build)

- Total Windows build time is 24min 49s
  - New initial Build Step (Docker Compose build, in parallel, which populates the Docker Engine cache): 13:26:36 -> 13:34:46 => 8 min 10s
  - The (legacy) build step (sequential docker builds, kept to be sure that nothing is broken): 13:34:46 -> 13:36:16 => 1 min 30
  - The test step (sequential test harnesses):13:36:16 -> 13:51:19 => 15min 03

@dduportal
Copy link
Contributor Author

Last build was 26 min (~ 14 % )

@timja
Copy link
Member

timja commented Oct 13, 2021

Do we need that legacy build step?

@dduportal
Copy link
Contributor Author

Do we need that legacy build step?

We could remove it, but it would side-track the work here: we would have to update the build.ps1 script to ensure that the docker image name from docker-compose's build are passed to tests.
Since it also manages the tags, I would prefer keeping the existing for now (in favor for a separated change of the script later) to not break builds on trusted.ci

@timja timja merged commit a2f3674 into jenkinsci:master Oct 13, 2021
@timja
Copy link
Member

timja commented Oct 13, 2021

Tests are failing on windows on trusted CI: https://trusted.ci.jenkins.io:1443/blue/organizations/jenkins/Containers%2Fdocker%2Fdocker-agent/detail/4.10-7/2/pipeline

@dduportal dduportal deleted the feat/full-adoptium branch October 14, 2021 14:57
@dduportal
Copy link
Contributor Author

@timja Interesting, the error is not related to the change here, but to the "Pester module" that could not be loaded in Windows. I'm digging to check why.

@dduportal
Copy link
Contributor Author

@timja
Copy link
Member

timja commented Oct 14, 2021

cool thanks

@timja
Copy link
Member

timja commented Oct 14, 2021

I tried twice before pinging here :p

@dduportal
Copy link
Contributor Author

I tried twice before pinging here :p

Yeah, that is what I saw, don't worry for that. Network issues ;)
I'm thinking about adding Pester and bats in the packer-images template for the future.

@timja
Copy link
Member

timja commented Oct 16, 2021

Weird errors occurring when publishing the next version:
https://trusted.ci.jenkins.io:1443/blue/organizations/jenkins/Containers%2Fdocker%2Fdocker-agent/detail/4.11-1/2/pipeline/9


[2021-10-16T16:56:07.279Z] docker-compose version 1.29.2, build 5becea4c

[2021-10-16T16:56:07.279Z] BUILD: Starting Docker Compose General Build

[2021-10-16T16:56:08.668Z] powershell.exe : Flag '--parallel' is ignored when building with COMPOSE_DOCKER_CLI_BUILD=1

[2021-10-16T16:56:08.668Z] At C:\Jenkins\workspace\iners_docker_docker-agent_4.11-1@tmp\durable-9f6b78b3\powershellWrapper.ps1:3 char:1

[2021-10-16T16:56:08.668Z] + & powershell -NoProfile -NonInteractive -ExecutionPolicy Bypass -Comm ...

[2021-10-16T16:56:08.668Z] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[2021-10-16T16:56:08.668Z]     + CategoryInfo          : NotSpecified: (Flag '--paralle...KER_CLI_BUILD=1:String) [], RemoteException

[2021-10-16T16:56:08.668Z]     + FullyQualifiedErrorId : NativeCommandError

and

2021-10-16T17:03:42.911Z] x509: certificate signed by unknown authority

all over the place =/

@timja
Copy link
Member

timja commented Oct 16, 2021

Passed on the third attempt

@dduportal
Copy link
Contributor Author

The x509 error is weird. The other error is expected: it's a report of the docker-compose command failing, but powershell shows the top level error, and then prints the message from subcommand.

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

Successfully merging this pull request may close these issues.

2 participants