-
Notifications
You must be signed in to change notification settings - Fork 160
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
tests/content-origins: Verify expected vendor for all RPM packages #1947
Conversation
case "${ID}" in | ||
fedora) vendor='Fedora Project';; | ||
rhel|rhcos) vendor='Red Hat, Inc.';; | ||
*) echo "Unknown operating system ID=${ID}; skipping this test"; exit 0;; |
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.
I wonder if we should actually error here if an unknown vendor pops up?
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.
You mean an unknown ID
right? We could, but it seems unnecessarily painful for e.g. someone who is creating a *coreos derivative and wants to run our tests.
I didn't want to scope in looking at c9s, partially because based on active discussions it seems highly likely we may end up e.g. using a COPR or a SIG repo at some point, and we don't need to be strict.
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.
I think that's fair.. one counter is to make this run on distros
of fcos
and rhcos
so this test won't even run on others?
still this is fine as is if we don't want to do 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.
The tricky thing is today kola makes distros=rhcos
run on centos stream https://github.com/coreos/coreos-assembler/blob/32e6d9f6664cfd05703d217e04f2c6f517e35f70/mantle/cmd/kola/options.go#L312
# Verify the RPM %{vendor} flag for everything installed matches what we expect. | ||
# | ||
# - platforms: qemu | ||
# - This test should pass everywhere if it passes anywhere. |
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.
I know you have some opposition to this so I'll say:
optional: mention exclusive=false here
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.
Right, my rationale against the duplicate comments is covered in coreos/coreos-assembler#3060 and coreos/coreos-assembler#3059
08b12b7
to
6b3447d
Compare
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.
LGTM
optional: maybe we should rename the directory so we have something like tests/kola/rpms/content-origins.sh
(nest under rpms
?
I don't have a strong opinion here. The idea of naming it "content-origins" was to be a bit independent of RPM, even though 98.2% of our content comes from RPMs. (Perhaps in the future we somehow verify the systemd units we pull from src/config/overlay*?). But yes OTOH it does only look at rpms today. |
As we move to support multiple streams (particularly for C9S versus RHEL) we should ensure that we're not accidentally including content from CentOS in official RHEL builds, and vice versa. The RPM vendor field is useful for this. For now, this test just verifies Fedora and RHEL vendors, and I tested on both.
6b3447d
to
a68ce11
Compare
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.
LGTM
As we move to support multiple streams (particularly for C9S versus RHEL)
we should ensure that we're not accidentally including content
from CentOS in official RHEL builds, and vice versa. The RPM vendor
field is useful for this.
For now, this test just verifies Fedora and RHEL vendors, and I
tested on both.