Skip to content

report correct version when multiple images invoked #327

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

Merged
merged 1 commit into from
Jan 5, 2023
Merged

report correct version when multiple images invoked #327

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion collection-scripts/gather
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# generate /must-gather/version file
. version
echo "openshift/must-gather" > /must-gather/version
echo "openshift/must-gather"> /must-gather/version
version >> /must-gather/version

# Named resource list, eg. ns/openshift-config
Expand Down
18 changes: 5 additions & 13 deletions collection-scripts/version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
#!/usr/bin/env bash

function version() {
# get version from image
version=$( \
oc status | grep '^pod' | \
sed -n -r -e 's/.*([[:digit:]]+\.[[:digit:]]+(:?\.[[:digit:]])?(:?-[^@]+)?).*/\1/p' \
)

# if version not found, fallback to imageID
[ -z "${version}" ] && version=$(oc status | grep '^pod.*runs' | sed -r -e 's/^pod.*runs //')

# if version still not found, use Unknown
[ -z "${version}" ] && version="Unknown"

echo ${version}
if [[ ! -z $OS_GIT_VERSION ]] ; then
Copy link
Member

Choose a reason for hiding this comment

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

OS_GIT_VERSION is defined only during the building phase. Once built, the env gets unpopulated. What about to inject the env value in https://github.com/openshift/must-gather/blob/master/Dockerfile.rhel7? E.g. re-generating the version file from scratch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The following are embedded into the images during build, and as such are available when the image is run:

'OS_GIT_MAJOR': major_version,
'OS_GIT_MINOR': minor_version,
'OS_GIT_PATCH': patch_version,
'OS_GIT_VERSION': f'{major_version}.{minor_version}.{patch_version}{release_suffix}',
'OS_GIT_TREE_STATE': 'clean',
'SOURCE_GIT_TREE_STATE': 'clean',
'BUILD_VERSION': version,
'BUILD_RELEASE': release if release else '',

You can download the must-gather.tar from one of the jobs on this PR, and see that the version file contains:

openshift/must-gather
4.13.0-202211071433.p0.g63e9a04.assembly.stream-63e9a04

Copy link
Member

@ingvagabund ingvagabund Dec 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Following https://github.com/openshift/enhancements/blob/master/enhancements/oc/must-gather.md#must-gather-images and the version string major.minor.micro.qualifier. I wonder what the expected mapping should be: major=4, minor=13, micro=0-202211071433, qualifier=p0.g63e9a04.assembly.stream-63e9a04? Compared to https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_must-gather/327/pull-ci-openshift-must-gather-master-images/1597782051325480960/artifacts/ci-operator-step-graph.json:

  • OS_GIT_MAJOR=4
  • OS_GIT_MINOR=13
  • OS_GIT_PATCH=0
  • BUILD_RELEASE=202211071433.p0.g63e9a04.assembly.stream

We might as well update the version string in enhancements/oc/must-gather.md to major.minor.micro-qualifier with major/minor/micro as [0-9]+ so the parts turn into major=4, minor=13, micro=0, qualifier=202211071433.p0.g63e9a04.assembly.stream-63e9a04.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ingvagabund I've updated the enhancement to specify the SemVer format (original intention) used elsewhere in OpenShift (vs. the OSGi-type format specified there originally).

Copy link
Member

Choose a reason for hiding this comment

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

With openshift/enhancements#1294 merged we are good here. @soltysh would you please approve the PR?

echo "${OS_GIT_VERSION}"
else
echo "0.0.0-unknown"
fi
}