-
Notifications
You must be signed in to change notification settings - Fork 163
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
build: DO NOT MERGE use rules_distroless #4710
base: master
Are you sure you want to change the base?
Conversation
Katya, I know it is only one of the issues, but I think your tester image is missing at least one package: iputils-ping. |
- "amd64" | ||
- "arm64" | ||
|
||
packages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference:
We have the following in our tester images:
packages:
- "bridge-utils"
- "curl"
- "fping"
- "iperf3"
- "iproute2"
- "iptables"
- "iputils-ping"
- "net-tools"
- "netcat-openbsd"
- "openssh-client"
- "openssh-server"
- "procps"
- "tcpdump"
- "telnet"
- "tshark"
- "wget"
- "nano"
- "vim"
Sadly for some of them, there needs to be some symlink shenanigans:
symlinks(
name = "tester_layer_deb_symlinks",
links = {
"./usr/bin/telnet": "/usr/bin/inetutils-telnet",
"./usr/bin/vim": "/usr/bin/vim.basic",
"./usr/bin/vi": "/usr/bin/vim.basic",
},
)
With:
def symlinks(name, links):
tar(
name = name,
mtree = [
"%s type=link link=%s mode=0755 uid=0 gid=0" % (src, dst)
for src, dst in links.items()
],
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After adding:
- "@tester_deb//iputils-ping",
- "@tester_deb//iproute2",
- "@tester_deb//net-tools",
All tests pass on my machine. There might still be something funny with the CI system though. I have not tested that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding these three packages solves the problem that I had locally too: all tests are passing now (two were failing before). But CI still doesn't see sh for some reason, hence other tests are failing. Lukas suggests upgrading the elastic CI stack, I'll take a look into it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @oncilla)
- "amd64" | ||
- "arm64" | ||
|
||
packages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding these three packages solves the problem that I had locally too: all tests are passing now (two were failing before). But CI still doesn't see sh for some reason, hence other tests are failing. Lukas suggests upgrading the elastic CI stack, I'll take a look into it
This reverts commit 54f05f5.
No description provided.