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

ci(jenkins): support windows build #1393

Merged
merged 129 commits into from
Mar 3, 2020
Merged

Conversation

v1v
Copy link
Member

@v1v v1v commented Sep 25, 2019

Highlights

  • Support testing in windows for the Node.js version 12.
  • appveyor is not supported anymore.

Iterations

  • Support docker in Windows. got some issues when using linux docker containers. no matching manifest for windows/amd64 in the manifest list entries

Tasks

  • Validate it works with versions: 10 and 12
  • Validate it works with the same list of versions stated in: .ci/.jenkins_nodejs.yml
  • Revert a37263c
  • process.title does not match the value in the CI windows workers #1418 should be fixed
  • Required to install ES, and other services as it was initially configured with.
  • If possible, let's bump a Linux worker with all the required docker containers and configure the Windows CI Worker with those env variables to be able to use those services out of the box. Then it might simplify dramatically the Windows environment...

Follow-ups

  • Transform windows test output to JUnit trends within the CI Pipeline. Let's work on this in a second iteration
  • When merged then it's required to disable the GH check for continuous-integration/appveyor/pr
  • Support testing in windows for all the Node.js versions defined in .ci/.jenkins_nodejs.yml
    • the toplevel linux agent is the one with all the services up and running, so can it be used with concurrent test executions?
  • Generate docker images in the internal docker registry to just consume them.
  • Docker orchestration with docker-compose.

Deprecated

Docker for windows is not required

See c21dc48

  • Docker images for:
    • cassandra.
    • elasticsearch.
    • mongodb.
    • mssql.
    • mysql.
    • postgres.
    • redis.

@v1v v1v force-pushed the feature/windows-build branch from a37263c to 2fc0dfd Compare September 25, 2019 14:24
@Qard
Copy link
Contributor

Qard commented Sep 25, 2019

Nice! Looking forward to seeing this land. :)

Also, we can probably drop AppVeyor in this PR too. What do you think, @watson?

@watson
Copy link
Contributor

watson commented Sep 25, 2019

Good idea @Qard - let's just remove AppVeyor in the PR as well 👍

Copy link
Contributor

@Qard Qard left a comment

Choose a reason for hiding this comment

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

LGTM other than minor comment about version pinning.

Jenkinsfile Outdated Show resolved Hide resolved
@watson
Copy link
Contributor

watson commented Sep 27, 2019

Does this need to be backported to the 2.x branch as well, or does it just need to be in master to also run Windows tests for PRs to and when new commits land in the 2.x branch`?

@Qard
Copy link
Contributor

Qard commented Sep 27, 2019

I would try to backport it, if we can. Getting appveyor fully out of the pipeline in all actively maintained branches would be ideal.

@v1v v1v changed the title 🚧 ci(jenkins): support windows build ci(jenkins): support windows build Sep 30, 2019
mdelapenya
mdelapenya previously approved these changes Dec 16, 2019
Copy link
Contributor

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

LGTM 👍

image

.ci/Jenkinsfile Show resolved Hide resolved
.ci/scripts/windows/install-tools.ps1 Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
@v1v v1v removed the in progress label Jan 7, 2020
@cla-checker-service
Copy link

❌ Author of the following commits did not sign a Contributor Agreement:
126da47, 86210cc

Please, read and sign the above mentioned agreement if you want to contribute to this project

@v1v v1v requested review from a team and watson and removed request for ElWPenn March 3, 2020 13:17
Copy link
Contributor

@watson watson left a comment

Choose a reason for hiding this comment

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

🎉 All looks good 👍 Just have a small question that's probably very easy to answer

.gitignore Show resolved Hide resolved
Copy link
Contributor

@watson watson left a comment

Choose a reason for hiding this comment

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

Great work!!

@watson
Copy link
Contributor

watson commented Mar 3, 2020

The two CI failures can safely be ignored IMO as long as you squash-and-merge and give the resulting squashed commit a valid commit message. If you prefer to verify this beforehand, you can squash locally and force push up to your feature branch of course.

@v1v v1v merged commit 340d223 into elastic:master Mar 3, 2020
@v1v v1v deleted the feature/windows-build branch March 3, 2020 15:00
@zube zube bot added [zube]: Done and removed [zube]: Inbox labels Mar 3, 2020
v1v added a commit to v1v/apm-agent-nodejs that referenced this pull request Mar 4, 2020
v1v added a commit that referenced this pull request Mar 4, 2020
@v1v v1v mentioned this pull request Mar 29, 2023
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.

6 participants