-
Notifications
You must be signed in to change notification settings - Fork 192
Use image SHA in make deploy
to avoid manual restarts
#1995
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
base: main
Are you sure you want to change the base?
Conversation
Currently, when iterating on changes in order to re-deploy the operator manager, a restart is required because `make deploy` uses image tags, with this change, there is no need to rollout restart the deployment and it will immediatelly be effective while being no-op when the image SHA is equal. Signed-off-by: Pierangelo Di Pilato <[email protected]>
WalkthroughThe Makefile was updated to add support for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant Skopeo
participant Kustomize
User->>Makefile: make manager-kustomization
Makefile->>Skopeo: skopeo inspect <image>
Skopeo-->>Makefile: Return image digest
Makefile->>Kustomize: kustomize edit set image <image>@<digest>
Poem
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 (
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Makefile
(5 hunks)README.md
(1 hunks)
🔇 Additional comments (4)
Makefile (3)
61-63
: Introduce SKOPEO binary path.Adding
SKOPEO ?= $(LOCALBIN)/skopeo
centralizes the binary location alongside other tools. This prepares the Makefile for the skopeo-based digest resolution.
72-74
: Pin SKOPEO version.Defining
SKOPEO_VERSION ?= v1.17.0
ensures consistent installs and reproducible builds.
459-467
: Extend go-install-tool to accept extra flags.The
go-install-tool
macro now supports an optional$4
parameter for additionalgo install
flags (e.g.,-tags="..."
), which is needed for installing skopeo with specific build tags. Verify that existing tool installs remain unaffected and that multi-flag quoting is preserved correctly.README.md (1)
186-187
: Fix extraneous quote in example command.Removing the stray double-quote ensures the usage example (
make image-build USE_LOCAL=true
) is syntactically correct and aligns with the updated Makefile.🧰 Tools
🪛 LanguageTool
[uncategorized] ~186-~186: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...setUSE_LOCAL
make variable totrue
e.gmake image-build USE_LOCAL=true
####...(E_G)
# phony target for the case of changing IMG variable | ||
.PHONY: manager-kustomization | ||
manager-kustomization: config/manager/kustomization.yaml.in | ||
manager-kustomization: skopeo config/manager/kustomization.yaml.in | ||
cd config/manager \ | ||
&& cp -f kustomization.yaml.in kustomization.yaml \ | ||
&& $(KUSTOMIZE) edit set image controller=$(IMG) | ||
&& $(KUSTOMIZE) edit set image controller=$$(skopeo inspect --retry-times=10 --no-tags docker://$(IMG) | jq -r '"\(.Name)@\(.Digest)"') | ||
|
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
Pin image by digest using skopeo inspect.
Using skopeo inspect
piped to jq
correctly resolves the image to its SHA digest, enabling deploys without manual restarts. However, this step depends on jq
being installed. Consider adding jq
to the tool binaries (with its own install target) or documenting its requirement to prevent failures in environments lacking jq
.
🤖 Prompt for AI Agents
In the Makefile around lines 249 to 255, the use of `jq` in the
`manager-kustomization` target requires `jq` to be installed, which may cause
failures if it's missing. To fix this, either add `jq` to the list of tool
binaries with an appropriate install target in the Makefile or update the
documentation to clearly state that `jq` is a prerequisite for running this
target, ensuring users are aware of this dependency.
/test opendatahub-operator-e2e |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1995 +/- ##
=======================================
Coverage 35.77% 35.77%
=======================================
Files 80 80
Lines 7681 7681
=======================================
Hits 2748 2748
Misses 4681 4681
Partials 252 252 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Currently, when iterating on changes in order to re-deploy the
operator manager, a restart is required because
make deploy
uses image tags, with this change, there is no need to rollout
restart the deployment and it will immediatelly be effective
while being no-op when the image SHA is equal.
How Has This Been Tested?
USE_LOCAL=true make image-build image-push deploy
Merge criteria
Summary by CodeRabbit
New Features
Chores
skopeo
tool to streamline image reference management.Documentation