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

tools/diff-kernel-config: various small improvements #3299

Merged
merged 3 commits into from
Jul 28, 2023
Merged

tools/diff-kernel-config: various small improvements #3299

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42d9840
tools/diff-kernel-config: Fix usage text to reflect reality
foersleo Jul 27, 2023
dfac598
tools/diff-kernel-config: Add release number to full kernel version
foersleo Jul 27, 2023
d7c90b2
tools/diff-kernel-config: Add resume functionality
foersleo Jun 20, 2023
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
17 changes: 13 additions & 4 deletions tools/diff-kernel-config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ bail() {

usage() {
cat <<EOF
Usage: $0 -b GITREV_BEFORE -a GITREV_AFTER -o OUTPUT_DIR [-k KERNEL_VERSION] [-h]
Usage: $0 -b GITREV_BEFORE -a GITREV_AFTER -o OUTPUT_DIR [-v VARIANT] [-r] [-h]
Compare kernel configurations before and after a series of commits.

-a, --after new Git revision to compare from
Expand All @@ -40,6 +40,8 @@ Compare kernel configurations before and after a series of commits.
be given multiple times (optional, defaults to this list:
'aws-k8s-1.23', 'metal-k8s-1.23', 'aws-dev', 'metal-dev')
-o, --output-dir path to the output directory; must not exist yet
-r, --resume resume work on a previous invocation; check which parts
already exist in OUTPUT_DIR and skip builds accordingly
-h, --help show this help text

Example invocation:
Expand Down Expand Up @@ -84,6 +86,8 @@ while [[ $# -gt 0 ]]; do
shift; variants+=( "$1" ) ;;
-o|--output-dir)
shift; output_dir=$1 ;;
-r|--resume)
shift; resume=1 ;;
-h|--help)
usage; exit 0 ;;
*)
Expand All @@ -102,7 +106,7 @@ done
readonly variants

[[ -n ${output_dir} ]] || usage_error 'require -o|--output-dir'
[[ -e ${output_dir} ]] && bail "Output directory '${output_dir}' exists already, not touching it"
[[ -e ${output_dir} && ! -v resume ]] && bail "Output directory '${output_dir}' exists already, not touching it"
readonly output_dir

# Validate and resolve the given before and after Git revisions. Resolving
Expand Down Expand Up @@ -163,6 +167,12 @@ for state in after before; do
kernel_versions+=( "${kver}" )

for arch in "${arches[@]}"; do
config_path=${output_dir}/config-${arch}-${variant}-${state}

if [[ -v resume && -e ${config_path} ]]; then
echo "${config_path} already exists, skipping"
continue
fi

debug_id="state=${state} arch=${arch} variant=${variant} kernel=${kver}"

Expand Down Expand Up @@ -206,13 +216,12 @@ for state in after before; do
;;
esac

kver_full=$(rpm --query --queryformat '%{VERSION}' "${kernel_rpm}")
kver_full=$(rpm --query --queryformat '%{VERSION}-%{RELEASE}' "${kernel_rpm}")
Copy link
Member

Choose a reason for hiding this comment

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

This makes sense for completeness. Only wondering out loud: This will report the release field of Bottlerocket's RPM and not the upstream one's, right? We would have to bump the release ourselves. I've seen we've at least done that for the 5.10 series at least once, but probably have not followed it closely (or the case hasn't come up yet for the 5.15 and 6.1 kernel series).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct. It probably does not make sense then as it is right now.
I will probably pull that commit from the PR then and think how we want to do this properly.

Copy link
Member

Choose a reason for hiding this comment

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

I'd still leave it in. We have bumped the release in the past and in the best case we'll be vigilant and do so again in the future when warranted. As an aside, I've been thinking about changes to our kernel version numbering scheme recently. I'll take this case into consideration.


#
# Extract kernel config
#

config_path=${output_dir}/config-${arch}-${variant}-${state}
rpm2cpio "${kernel_rpm}" \
| cpio --quiet --extract --to-stdout ./boot/config >"${config_path}"
[[ -s "${config_path}" ]] || bail "Failed to extract config for ${debug_id}"
Expand Down