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

ironbank: validate heartbeat docker context #32982

Merged
merged 10 commits into from
Sep 7, 2022

Conversation

v1v
Copy link
Member

@v1v v1v commented Sep 5, 2022

What does this PR do?

It validates the DoD docker context for x-pack/heartbeat contains the required dependencies and the docker image can be built.

Why is it important?

As agreed, docker context generation does not validate the changes, hence this is the approach to verify the artifacts to be added to the DoD docker context are built correctly.

Test

CI

See this build

image

image

NOTE: The existing pipeline will not run on PR basis, but on a time basis for the active branches (see #32977), though users can trigger a build manually using the Jenkins UI.

Manually

hub pr checkout 32982
cd x-pack/heartbeat
make -C ironbank package
mage ironbank
make -C ironbank validate-ironbank
echo $?

produced:

curl -sSfL -o ../build/heartbeat-ironbank-8.5.0-docker-build-context/tinit https://github.com/krallin/tini/releases/download/v0.19.0/tini-amd64
curl -sSfL -o ../build/heartbeat-ironbank-8.5.0-docker-build-context/jq https://github.com/stedolan/jq/releases/download/jq-1.6/jq-linux64
...
...
docker build \
		--build-arg BASE_REGISTRY=docker.elastic.co \
		--build-arg BASE_IMAGE=ubi8/ubi \
		--build-arg BASE_TAG=latest \
		../build/heartbeat-ironbank-8.5.0-docker-build-context
 => exporting to image                                                     0.0s
 => => exporting layers                                                    0.0s
 => => writing image sha256:acd39886d84caaacc713628341380cbeb430866f758ba  0.0s
...
...
0

Follow ups

Potentially it could be also tested with some integration testing hence the docker heartbeat works with the Elastic Stack. Although to do so, it requires to store the docker-compose for the Elastic Stack and some users/roles (similar to https://github.com/jenkinsci/opentelemetry-plugin/tree/master/src/test/resources).
For the time being, I'll say to keep it simple, otherwise we can add certain complexity that might not be needed

Issues

Requires #32976

@v1v v1v added automation Team:Automation Label for the Observability productivity team backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.4.0 Automated backport with mergify labels Sep 5, 2022
@v1v v1v self-assigned this Sep 5, 2022
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 5, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 5, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-09-06T11:14:03.967+0000

  • Duration: 46 min 53 sec

Test stats 🧪

Test Results
Failed 0
Passed 178
Skipped 0
Total 178

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Comment on lines +46 to +48
failure {
notifyStatus(slackStatus: 'danger', subject: "[${env.REPO}@${BRANCH_NAME}] package for ${env.BEATS_FOLDER}", body: "Contact the heartbeats team. (<${env.RUN_DISPLAY_URL}|Open>)")
}
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 failure might be relevant to the product team, since it's related to the packaging

Comment on lines +61 to +63
failure {
notifyStatus(slackStatus: 'danger', subject: "[${env.REPO}@${BRANCH_NAME}] Ironbank docker context for ${env.BEATS_FOLDER}", body: "Contact the @observablt-robots-team team. (<${env.RUN_DISPLAY_URL}|Open>)")
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Ironbank packages could fail, if so, likely an environmental issue, since the DRA will also fail. In any case we will redirect the message to the robots team

Comment on lines +76 to +78
failure {
notifyStatus(slackStatus: 'danger', subject: "[${env.REPO}@${BRANCH_NAME}] Ironbank validation failed", body: "Contact the @observablt-robots-team team. (<${env.RUN_DISPLAY_URL}|Open>)")
}
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 the new validation, so if the docker image cannot be built then something is broken in the list of dependencies, if that happens then we will need to fix it.

@v1v v1v requested review from kuisathaverat and a team September 6, 2022 11:14
@v1v v1v marked this pull request as ready for review September 6, 2022 11:14
@v1v v1v requested review from a team as code owners September 6, 2022 11:14
@v1v v1v requested review from fearful-symmetry and faec and removed request for a team September 6, 2022 11:14
@amannocci amannocci self-requested a review September 6, 2022 16:52
Copy link
Contributor

@amannocci amannocci left a comment

Choose a reason for hiding this comment

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

LGTM


## @help:setup-yq: Install yq in ../build/yq.
.PHONY: setup-yq
setup-yq:
Copy link
Contributor

Choose a reason for hiding this comment

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

We could try to detect if yq is installed at the OS level and if not then download it.

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'd prefer to keep the logic simpler and agnostic to the worker, we can revisit this in the future if needed

@v1v v1v merged commit 7cbb828 into elastic:main Sep 7, 2022
mergify bot pushed a commit that referenced this pull request Sep 7, 2022
mergify bot pushed a commit that referenced this pull request Sep 7, 2022
v1v added a commit that referenced this pull request Sep 7, 2022
(cherry picked from commit 7cbb828)

Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
v1v added a commit that referenced this pull request Sep 7, 2022
(cherry picked from commit 7cbb828)

Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.4.0 Automated backport with mergify Team:Automation Label for the Observability productivity team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants