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): enable pipeline #8

Merged
merged 6 commits into from
Nov 26, 2019
Merged

ci(jenkins): enable pipeline #8

merged 6 commits into from
Nov 26, 2019

Conversation

v1v
Copy link
Member

@v1v v1v commented Nov 22, 2019

Tasks

  • Enable JJBB to create the Job
  • Multibranch Pipeline with a basic build stage.
  • Enable docker environment for building in the CI.

Further details.

Docker container will run the make default goal, then the CI pipeline will orchestrate the remaining goals in some sequential stages to reuse the same workspace. Remaining goals are:

  • make test
  • make install

The only output which it's generated at the moment, besides the log output, is the test output in JUnit format to be digested by the CI pipeline in Jenkins (see the below screenshots).

The main reason to use docker is to avoid setting up the local environment and be deterministic about what are the tools required to be used for building/testing and installing the library. It's defined once and the CI build can be easily reproduced locally with docker.

e2e tests

  • Pipeline view

image

  • Test output overview

image

v1v added a commit to v1v/apm-integration-testing that referenced this pull request Nov 22, 2019
@v1v v1v marked this pull request as ready for review November 25, 2019 09:53
@v1v v1v self-assigned this Nov 25, 2019
@v1v v1v added automation ci enhancement New feature or request labels Nov 25, 2019
RUN apk --no-cache add build-base autoconf curl-dev

WORKDIR /app/src/ext

Copy link
Contributor

Choose a reason for hiding this comment

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

we need to copy the extension to /app/src/ext first in order compile "it" on L:7

COPY src/ext /app/src/ext

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not required since this particular docker image is only for the environmental requirements.
The docker container should mount the volume and therefore the CMD directive will run those commands using the mounted volume.

Dockerfile Outdated
CMD phpize \
&& ./configure --enable-elasticapm \
&& make clean \
&& make \
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add && make test \

Copy link
Member Author

Choose a reason for hiding this comment

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

I was not sure whether either make or make install might call the command make test as a dependent goal. I'll put it explicitly, now, and archive the test output

Copy link
Member Author

Choose a reason for hiding this comment

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

I've decided to change the approach:

The reason is to help to categorize those commands in stages in the pipeline and visualise them easily in the CI.

Dockerfile Outdated Show resolved Hide resolved
… Besides, some readme details about using the docker container
ENV REPORT_EXIT_STATUS=1
ENV TEST_PHP_DETAILED=1
ENV NO_INTERACTION=1
ENV TEST_PHP_JUNIT=/app/junit.xml
Copy link
Member Author

Choose a reason for hiding this comment

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

This env variable and the above ones are required when running the make test to help with:

  • More detailed info
  • Report the exit
  • Avoid interactive when a test failure to send an email to the PHP community
  • Generate junit output in a file to be consumed within the Jenkins pipeline.

@v1v
Copy link
Member Author

v1v commented Nov 26, 2019

I'll proceed to merge this PR and we can iterate later on with some follow ups :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation ci enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants