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

test: check package versions and downgrades #609

Closed
wants to merge 1 commit into from
Closed

test: check package versions and downgrades #609

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions tests/kola/package/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#!/bin/bash
set -xeuo pipefail

# kola: { "architectures": "!s390x ppc64le", "minMemory": 2048, "tags": "needs-internet" }

ok() {
echo "ok" "$@"
exit 0
}

fatal() {
echo "$@"
exit 1
}

# Verify all rhaos packages contain the same OpenShift version
test_package_versions() {
if [[ $(rpm -qa | grep rhaos | grep -v $OPENSHIFT_VERSION) ]]; then
fatal "Error: rhaos packages do not match OpenShift version"
Copy link
Member

Choose a reason for hiding this comment

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

Do we care if this function causes the test to exit and not do the downgrade check? Or should the test try to accumulate errors?

Copy link
Member Author

@mike-nguyen mike-nguyen Aug 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

accumulating errors is probably better so we catch everything but we don't seem to do it on any of our tests

fi
}

# Verify there are no downgraded packages
test_downgraded_packages() {
RELEASE=$OPENSHIFT_VERSION
STREAM=fast-$RELEASE
GRAPH=$(curl -sfH "Accept:application/json" "https://api.openshift.com/api/upgrades_info/v1/graph?channel=$STREAM")
if [[ $? -ne 0 ]]; then
fatal "Unable to get graph"
fi
Comment on lines +27 to +30
Copy link
Member

@miabbott miabbott Aug 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Interestingly, if you provide a non-existent channel as an arg to that URL, it will still return a result:

$ curl -sfH Accept:application/json 'https://api.openshift.com/api/upgrades_info/v1/graph?channel=foobar'
{"nodes":[],"edges":[]}
$ echo $?
0

So unless the infra is down, your return code is always going to be a success. Maybe you want to do some additional check that the content returned isn't empty? (edit: I see the check below now)

Also, since the script uses set -e, if curl does return a failure the whole script will exit, so you don't need to check the return code.


# There are no released builds on master so no need to check downgraded packages
if [[ $(echo $GRAPH | jq 'has("nodes") and has("edges") and (.nodes | length == 0)') =~ "true" ]]; then
ok "No released stream"
fi

# The cincinatti graph defines nodes as a list of objects and edges as a list
# of list of two integers. Nodes are releases, and edges are updates.
# [x, y] is [from release index, to release index]. The release index can
# change every request for the cincinatti graph!
#
# Use jq to find the node that contains no "from release index" edge.
# 1. Find all the unique "from release indexes"
# 2. Find all the release indexes
# 3. Get the difference between all the releases and the "from release
# indexes". This is the latest because there is no update from this
# release.
PAYLOAD=$(echo $GRAPH | jq -r '. as $graph | [$graph.edges[][0]] | unique as $from | $graph.nodes | to_entries as $indexed | [$indexed[].key] | unique as $nodes | ($nodes - $from)[] as $latest | $indexed[] | select(.key == $latest) | .value.payload')
Copy link
Member

Choose a reason for hiding this comment

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

This is some veritable jq sorcery.

VERSION=$(oc adm release info $PAYLOAD -o json | jq -r '.displayVersions."machine-os".Version')
OCP_COMMIT=$(curl -sSL https://art-rhcos-ci.s3.amazonaws.com/releases/rhcos-$RELEASE/$VERSION/x86_64/meta.json | jq -r '."ostree-commit"')

curl -SL https://art-rhcos-ci.s3.amazonaws.com/releases/rhcos-$RELEASE/$VERSION/x86_64/rhcos-$VERSION-ostree.x86_64.tar -o $STREAM.tar
Copy link
Member

Choose a reason for hiding this comment

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

This will break after we try again with #593

I think it'd be a bit cleaner to download the meta.json and then do jq .images.ostree.path on it, then we can conditionalize on whether it ends in .ociarchive or .tar as is done in e.g.
https://github.com/coreos/coreos-assembler/blob/88efaff63b38542763336e12249b11f53d606776/src/cosalib/cmdlib.py#L251

Also avoid duplicating the URL would be good.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cgwalters it looks like the version of rpm-ostree in the latest rhcos does not have the ex-container functionality. I see that you converted the oci-archive to tar inside cosa before and copied it to the test system [0] but this is an external test, and I don't think that's possible. Do you have any ideas to work around this?

Some ideas:

  • create a ostree container to handle the conversion
  • rewrite test as a mantle kola test and do the same thing as [0]

I also saw that in the future the container may contain a http server. Would anything need to change in the oci-archive or would it just ab an additional http server to serve the ostree repo?

[0] https://github.com/coreos/coreos-assembler/blob/350c1d93bc3343c8f523d642664bccd8dc0424c9/mantle/kola/tests/rhcos/upgrade.go#L95

Copy link
Member

@cgwalters cgwalters Sep 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

There's more alternatives:

  • Add cosa build --disallow-downgrades and use it in the pipeline by default (and have a declarative file of allowed downgrades?)
  • Add the above, but implement in rpm-ostree (implicity adding allowed-downgrades: [] i.e. empty YAML list would disallow downgrades, then the list is an allowlist)

Copy link
Member

Choose a reason for hiding this comment

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

Either way, this is basically the same thing as coreos/fedora-coreos-config#892 right? So if we deduplicate in either coreos-assembler or rpm-ostree it would be a win (less pipeline code, more easily reproduced outside of pipelines, etc.)

I kind of like the allowed-downgrades in rpm-ostree natively but it's also pretty easy to implement outside it in a cosa workdir which is what the FCOS pipeline PR above is doing.

Copy link
Member Author

Choose a reason for hiding this comment

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

jlebon's comment may apply here because it would be checking from the last build and not the last release. Maybe this test should be scrapped and changed to an upgrade test from the previous releases and db diff instead of just doing a db diff.

Copy link
Member

Choose a reason for hiding this comment

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

I think going the cosa build --disallow-downgrades + YAML path could work. There'd be overhead involved, but maybe that's OK. I do like that with coreos/fedora-coreos-config#892, it's just an easy "rubber-stamping" thing: we don't have to do anything else than just force merge the PR. In the pipeline itself, nothing cares about downgrades.

For RHCOS, that's harder to do without lockfiles because unexpected downgrades could happen anytime up to and including during the prod pipeline build. So it's much harder to make it as lightweight as in FCOS.


mkdir -p repo && tar xvf $STREAM.tar -C $_ && rm -rf $STREAM.tar

ostree pull-local repo

RHCOS_COMMIT=$(rpm-ostree status --json | jq -r .deployments[0].checksum)

if [[ $(rpm-ostree db diff $OCP_COMMIT $RHCOS_COMMIT | grep -A1000 Downgraded) ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Hm, we should add something like db diff --downgrades or something so this test could just validate it's empty.

fatal "Error: downgraded packages were found."
fi
}


main() {
cd $(mktemp -d)
source /etc/os-release
test_package_versions
test_downgraded_packages
}

main