-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Added major and minor tags #6611
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
Conversation
📝 Walkthrough""" WalkthroughThe GitHub Actions workflow for Docker image building has been updated to extract the major and minor version numbers from the current Git tag. These values are set as outputs and used as environment variables during the Docker build process. The Docker image tagging logic is modified to include tags for the major version, minor version, the full version, and "latest" for all image variants, including regular, unprivileged, GPU NVIDIA, and GPU AMD builds. The tagging is applied to both Docker Hub and GitHub Packages repositories. No changes were made to exported or public code entities. Assessment against linked issues
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/docker-image.yml
(6 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/docker-image.yml
22-22: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
22-22: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
79-79: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
79-79: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🔇 Additional comments (5)
.github/workflows/docker-image.yml (5)
30-31
: Good use of version outputs as environment variables
InjectingMAJOR_VERSION
andMINOR_VERSION
into the build environment ensures the Docker tags can reference these values directly. This correctly wires up the new outputs for the regular build.
44-46
: Include major and minor tags for Docker Hub images
The addition ofDOCKERHUB_TAG_MINOR
,DOCKERHUB_TAG_MAJOR
, and inclusion in theTAGS
variable implements the PR objective of multi-level version tagging (major/minor/full/latest).
58-60
: Propagate major and minor tags to unprivileged variant
Correctly extendsTAGS_UNPRIVILEGED
with bothMINOR_VERSION
andMAJOR_VERSION
tags, mirroring the logic of the regular image build.
99-101
: Extend GPU NVIDIA builds with major/minor tags
Adds-t ${DOCKERHUB_TAG_MINOR}
and-t ${DOCKERHUB_TAG_MAJOR}
for the NVIDIA GPU variant, ensuring consistency with the versioning scheme.
113-115
: Extend GPU AMD builds with major/minor tags
Adds-t ${DOCKERHUB_TAG_MINOR}
and-t ${DOCKERHUB_TAG_MAJOR}
for the AMD GPU variant, matching the same tagging structure.
.github/workflows/docker-image.yml
Outdated
- name: get minor and major tags | ||
id: version | ||
run: | | ||
MAJOR_VERSION=$(echo "${{ steps.vars.outputs.tag }}" | cut -d '.' -f 1) | ||
MINOR_VERSION=$(echo "${{ steps.vars.outputs.tag }}" | cut -d '.' -f 1-2) | ||
echo ::set-output name=major_version::${MAJOR_VERSION} | ||
echo ::set-output name=minor_version::${MINOR_VERSION} |
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.
Fix deprecation and wire outputs into GPU build
- You extract
major_version
/minor_version
here but never export them for the GPU build step—those vars will be undefined at runtime. - Update deprecated
set-output
commands as above.
Within this block, apply:
- echo ::set-output name=major_version::${MAJOR_VERSION}
- echo ::set-output name=minor_version::${MINOR_VERSION}
+ echo "major_version=${MAJOR_VERSION}" >> $GITHUB_OUTPUT
+ echo "minor_version=${MINOR_VERSION}" >> $GITHUB_OUTPUT
Then, in the subsequent GPU build step, include the new outputs in its environment:
- - name: Build the Docker image
- env:
- RELEASE_VERSION: ${{ steps.vars.outputs.tag }}
+ - name: Build the Docker image
+ env:
+ RELEASE_VERSION: ${{ steps.vars.outputs.tag }}
+ MAJOR_VERSION: ${{ steps.version.outputs.major_version }}
+ MINOR_VERSION: ${{ steps.version.outputs.minor_version }}
🧰 Tools
🪛 actionlint (1.7.7)
79-79: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
79-79: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🤖 Prompt for AI Agents
In .github/workflows/docker-image.yml around lines 77 to 83, the script uses the
deprecated set-output command to export major_version and minor_version, and
these outputs are not wired into the GPU build step environment, causing them to
be undefined at runtime. Replace the set-output commands with the new syntax
using echo with the ::set-output replacement format, and then update the GPU
build step to include MAJOR_VERSION and MINOR_VERSION environment variables by
referencing them as ${{ steps.version.outputs.major_version }} and ${{
steps.version.outputs.minor_version }} respectively.
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.
Should I create an issue to update ::set-output seperately to use $GITHUB_OUTPUT instead of ::set-output?
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.
🧩 Analysis chain
@jayantpranjal0 That's a great approach! Separating the deprecation fix from your feature work keeps the PR focused and makes it easier to review.
Let me first check how widespread the ::set-output
usage is across the codebase, and then I can help you create an issue to track this technical debt separately.
🏁 Script executed:
#!/bin/bash
# Search for all instances of deprecated ::set-output in the codebase
rg "::set-output" -A 2 -B 2
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for set-output with different patterns
rg "set-output" -A 2 -B 2
# Also search more broadly for workflow commands
rg "echo.*::" -A 1 -B 1
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Look specifically at the file in question to confirm the pattern
cat .github/workflows/docker-image.yml | grep -n "set-output"
# Search for GitHub workflow files first
fd -e yml -e yaml .github/
# Then search within those workflow files for set-output patterns
find .github -name "*.yml" -o -name "*.yaml" | xargs grep -n "set-output" || echo "No set-output found in workflow files"
# Also check for other workflow command patterns
find .github -name "*.yml" -o -name "*.yaml" | xargs grep -n "echo.*::" || echo "No echo :: patterns found"
Length of output: 2172
@jayantpranjal0 Perfect! Your approach is definitely the right one. I found that there are 6 total instances of the deprecated ::set-output
command in this workflow file:
Existing code (pre-PR):
- Line 19:
echo ::set-output name=tag::${GITHUB_REF#refs/*/}
(in build job) - Line 76:
echo ::set-output name=tag::${GITHUB_REF#refs/*/}
(in build-gpu job)
New code (from your PR):
- Lines 25, 26: The new major/minor version outputs in build job
- Lines 82, 83: The new major/minor version outputs in build-gpu job
Since there are already existing deprecated usages, creating a separate issue makes total sense. You have two options:
- Recommended: Create an issue to update all 6 instances, and keep your current PR focused on the feature
- Update just your new code (lines 25, 26, 82, 83) in this PR and create an issue for the existing ones (lines 19, 76)
Would you like me to create the issue for you? If so, I can open it with a proper description covering all the deprecated ::set-output
usage that needs to be updated to use $GITHUB_OUTPUT
.
🐰
/| /|
( ._. )
o_(")(")
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 have fixed the issue of not using major and minor version tags in docker build properly. Recheck 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.
@jayantpranjal0 Excellent work! 🎉 I've rechecked the implementation and can confirm that you've properly fixed the issue.
✅ What's now working correctly:
- Both jobs properly extract versions: Both
build
andbuild-gpu
jobs now have the "Get minor and major tags" step - Environment variables properly wired: Both jobs now include
MAJOR_VERSION
andMINOR_VERSION
in their environment variables - Tags correctly used: All Docker build variants (regular, unprivileged, GPU NVIDIA, GPU AMD) now properly use
${MAJOR_VERSION}
and${MINOR_VERSION}
in their tag construction
Example of the correct tagging you've implemented:
# For regular builds
DOCKERHUB_TAG_MINOR="qdrant/qdrant:${MINOR_VERSION}"
DOCKERHUB_TAG_MAJOR="qdrant/qdrant:${MAJOR_VERSION}"
# For GPU builds
DOCKERHUB_TAG_MINOR="qdrant/qdrant:${MINOR_VERSION}-gpu-nvidia"
DOCKERHUB_TAG_MAJOR="qdrant/qdrant:${MAJOR_VERSION}-gpu-nvidia"
The only remaining item is the deprecated ::set-output
commands, but as we discussed, that's better handled in a separate issue to keep this PR focused on the feature.
Your implementation now correctly provides multi-level version tagging for all Docker image variants! 🐳
.github/workflows/docker-image.yml
Outdated
- name: get minor and major tags | ||
id: version | ||
run: | | ||
MAJOR_VERSION=$(echo "${{ steps.vars.outputs.tag }}" | cut -d '.' -f 1) | ||
MINOR_VERSION=$(echo "${{ steps.vars.outputs.tag }}" | cut -d '.' -f 1-2) | ||
echo ::set-output name=major_version::${MAJOR_VERSION} | ||
echo ::set-output name=minor_version::${MINOR_VERSION} |
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.
🛠️ Refactor suggestion
Update deprecated set-output
commands in version extraction
GitHub Actions has deprecated the echo ::set-output
syntax. Please switch to the $GITHUB_OUTPUT
file interface to future-proof this step and avoid warnings/breakages.
Apply this diff within the version step:
- echo ::set-output name=major_version::${MAJOR_VERSION}
- echo ::set-output name=minor_version::${MINOR_VERSION}
+ echo "major_version=${MAJOR_VERSION}" >> $GITHUB_OUTPUT
+ echo "minor_version=${MINOR_VERSION}" >> $GITHUB_OUTPUT
🧰 Tools
🪛 actionlint (1.7.7)
22-22: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
22-22: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🤖 Prompt for AI Agents
In .github/workflows/docker-image.yml around lines 20 to 26, the use of the
deprecated `echo ::set-output` commands to set output variables should be
replaced. Modify the script to write the output values to the file specified by
the environment variable `$GITHUB_OUTPUT` using the syntax `echo "name=value" >>
$GITHUB_OUTPUT` instead of `echo ::set-output name=name::value`. This change
will update the major_version and minor_version outputs accordingly and prevent
deprecation warnings.
6e26802
to
e82adfb
Compare
e82adfb
to
b8ab893
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.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/docker-image.yml (2)
20-26
: Update deprecated set-output syntax
Theecho ::set-output
commands are deprecated and will break on newer GitHub Actions runners. Replace them with the$GITHUB_OUTPUT
file interface as follows:- echo ::set-output name=major_version::${MAJOR_VERSION} - echo ::set-output name=minor_version::${MINOR_VERSION} + echo "major_version=${MAJOR_VERSION}" >> $GITHUB_OUTPUT + echo "minor_version=${MINOR_VERSION}" >> $GITHUB_OUTPUT🧰 Tools
🪛 actionlint (1.7.7)
22-22: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
22-22: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
77-83
: Update deprecated set-output syntax in GPU build step
The GPU build’s version extraction still uses the old::set-output
commands. Apply the same$GITHUB_OUTPUT
fix here to prevent deprecation issues:- echo ::set-output name=major_version::${MAJOR_VERSION} - echo ::set-output name=minor_version::${MINOR_VERSION} + echo "major_version=${MAJOR_VERSION}" >> $GITHUB_OUTPUT + echo "minor_version=${MINOR_VERSION}" >> $GITHUB_OUTPUT🧰 Tools
🪛 actionlint (1.7.7)
79-79: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
79-79: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
🧹 Nitpick comments (1)
.github/workflows/docker-image.yml (1)
44-46
: Consider using a matrix job to reduce duplication
The tag definitions for regular, unprivileged, GPU NVIDIA, and GPU AMD variants are almost identical. You can simplify maintenance and avoid copy-paste errors by leveraging a GitHub Actions matrix (e.g., definevariant
andsuffix
in a matrix and loop over tags).Also applies to: 58-60, 101-103, 115-117
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/docker-image.yml
(6 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/docker-image.yml
22-22: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
22-22: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
79-79: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
79-79: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT
instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🔇 Additional comments (2)
.github/workflows/docker-image.yml (2)
30-31
: Environment variables wired correctly for Docker build
The newMAJOR_VERSION
andMINOR_VERSION
outputs are correctly exposed to the build step’s environment. This ensures the subsequent tag logic can reference them without issues.
87-88
: Environment variables wired correctly for GPU Docker build
The GPU build step properly referencesMAJOR_VERSION
andMINOR_VERSION
from theversion
step outputs—no action needed here.
@timvisee I tried reproducing the error in test that failed, but it worked correctly for me. Can you help me out with what might be going wrong? |
@timvisee can you merge this PR? |
Thanks @jayantpranjal0 for pushing it! |
Co-authored-by: Jayant <jayant@Jayants-MacBook-Air.local>
All Submissions:
Closes #1957
dev
branch. Did you create your branch fromdev
?There is another open pull request, but it need changes and based of too old code.