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.

Sign up for GitHub

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

Jump to bottom

CI, docs: remove libelf-dev from dependencies #17687

Merged
merged 3 commits into from Nov 10, 2021
Merged

CI, docs: remove libelf-dev from dependencies #17687

merged 3 commits into from Nov 10, 2021

Conversation

tklauser
Copy link
Member

Since commit 4db3166 ("cilium-map-migrate: remove program and all
of its references") cilium-map-migrate has been replaced by a binary
written in Go and we no longer link binaries against libelf as of commit
c67cf7e ("bpf: remove libelf dependency and unused nobpf.h"). Thus,
drop libelf-dev from dependencies in CI and documentations.

Also update the cilium-builder image.

@tklauser tklauser added area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Oct 25, 2021
@tklauser tklauser requested review from a team as code owners October 25, 2021 09:01
@tklauser tklauser mentioned this pull request Oct 25, 2021
Closed
@tklauser tklauser force-pushed the pr/tklauser/libelf-dev-remove branch from bc23d7a to 8aecd2d Compare October 25, 2021 09:04
@tklauser tklauser force-pushed the pr/tklauser/libelf-dev-remove branch from 8aecd2d to 2f2a581 Compare October 25, 2021 09:08
@tklauser tklauser temporarily deployed to release-base-images October 25, 2021 09:08 Inactive
@tklauser tklauser temporarily deployed to release-base-images October 25, 2021 09:08 Inactive
@tklauser
Copy link
Member Author

/test

qmonnet
qmonnet reviewed Oct 25, 2021
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks good. I can also see references to (elfutils-)libelf-dev in the following files:

  • contrib/packaging/rpm/Dockerfile
  • contrib/packaging/rpm/cilium.spec.envsubst
  • contrib/ansible/roles/common/tasks/common_delete.yml
  • contrib/ansible/roles/common/tasks/common_install.yml

Should they go as well?

@tklauser
Copy link
Member Author

Looks good. I can also see references to (elfutils-)libelf-dev in the following files:

* contrib/packaging/rpm/Dockerfile

* contrib/packaging/rpm/cilium.spec.envsubst

* contrib/ansible/roles/common/tasks/common_delete.yml

* contrib/ansible/roles/common/tasks/common_install.yml

Should they go as well?

Thanks, removed them as well.

Initially, I wasn't sure about them because I had no way to test changes on these files and they don't seem to be covered by CI. It looks like they haven't been updated (apart from minor changes) in a while. Are they still used by people at all?

@tklauser tklauser force-pushed the pr/tklauser/libelf-dev-remove branch from 2f2a581 to a088a5f Compare October 25, 2021 10:00
@tklauser tklauser temporarily deployed to release-base-images October 25, 2021 10:00 Inactive
@tklauser
Copy link
Member Author

tklauser commented Nov 1, 2021

Rebased and added a second commit to remove ipset which was accidentially added to the Travis CI packages in #17706, see #17706 (comment)

@tklauser tklauser mentioned this pull request Nov 1, 2021
Merged
@tklauser
Copy link
Member Author

tklauser commented Nov 1, 2021

/test

@tklauser tklauser removed the request for review from glibsm November 5, 2021 07:48
@tklauser tklauser added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Nov 5, 2021
@tklauser
Copy link
Member Author

tklauser commented Nov 5, 2021

Will rebase and rebuild images once #17792 is merged.

@tklauser
CI, docs, contrib: remove libelf-dev from dependencies
b9f6c11 
Since commit 4db3166 ("cilium-map-migrate: remove program and all
of its references") cilium-map-migrate has been replaced by a binary
written in Go and we no longer link binaries against libelf as of commit
c67cf7e ("bpf: remove libelf dependency and unused nobpf.h"). Thus,
drop libelf-dev from dependencies in CI and documentation.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser
.travis.yml: don't install ipset
35375fe 
In commit b71d528 ("CI: update cilium-cli to v0.9.2"), ipset was
accidentially added to the list of packages to install on Travis CI.
Remove it again as it is not needed by the tests.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/libelf-dev-remove branch from e60a5b9 to 4d0942e Compare November 10, 2021 09:13
@tklauser tklauser temporarily deployed to release-base-images November 10, 2021 09:13 Inactive
@tklauser tklauser temporarily deployed to release-base-images November 10, 2021 09:13 Inactive
@tklauser
images: remove libelf-dev from cilium-builder
a59ab13 
We still need libelf1 as a dependency for iproute2 and runtime though.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/libelf-dev-remove branch from 4d0942e to a59ab13 Compare November 10, 2021 09:23
@tklauser tklauser removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Nov 10, 2021
@tklauser
Copy link
Member Author

tklauser commented Nov 10, 2021 •
edited by maintainer-s-little-helper bot

/test

Job 'Cilium-PR-K8s-GKE' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig Host firewall Check connectivity with IPv6 disabled

Failure Output

FAIL: Pods are not ready in time: timed out waiting for pods with filter  to be ready: 4m0s timeout expired

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-GKE so I can create a new GitHub issue to track it.

@tklauser
Copy link
Member Author

GKE failure looks like #17307: https://jenkins.cilium.io/job/Cilium-PR-K8s-GKE/6891/

@tklauser
Copy link
Member Author

test-gke

@tklauser tklauser mentioned this pull request Nov 10, 2021
Closed
@tklauser
Copy link
Member Author

All tests passed and reviews are in, marking as ready to merge.

@tklauser tklauser added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 10, 2021
@pchaigno pchaigno merged commit 85d80bc into master Nov 10, 2021
@pchaigno pchaigno deleted the pr/tklauser/libelf-dev-remove branch November 10, 2021 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@nebril nebril nebril approved these changes

@pchaigno pchaigno pchaigno approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees
No one assigned
Labels
area/CI Continuous Integration testing issue or flake ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@tklauser @joestringer @nebril @pchaigno @qmonnet @glibsm