Add operator and bundle build-push script#79
Open
shreyabiradar07 wants to merge 4 commits intokruize:mvp_demofrom
Open
Add operator and bundle build-push script#79shreyabiradar07 wants to merge 4 commits intokruize:mvp_demofrom
shreyabiradar07 wants to merge 4 commits intokruize:mvp_demofrom
Conversation
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Reviewer's GuideAdds a new Bash script to automate building, pushing, and validating Kruize operator and bundle images with aligned versioning and basic prerequisites checks. Sequence diagram for operator_build_and_push.sh interactions with toolssequenceDiagram
actor Developer
participant Script as operator_build_and_push_sh
participant Make as make
participant ContainerTool as CONTAINER_TOOL
participant OperatorSDK as operator_sdk_bin
participant FS as FileSystem
Developer->>Script: Run with -o OPERATOR_IMAGE -b BUNDLE_IMAGE
Script->>Script: Parse args and validate required images
Script->>Script: Extract OPERATOR_VERSION and BUNDLE_VERSION
Script->>Script: Compare versions
Script-->>Developer: Exit on mismatch
Script->>Script: check_prerequisites
Script->>ContainerTool: which CONTAINER_TOOL
ContainerTool-->>Script: Found or error
Script->>Make: which make
Make-->>Script: Found or error
Script->>FS: Check operator-sdk or ./bin/operator-sdk
alt operator-sdk missing
Script->>Make: make operator-sdk
Make-->>FS: Download ./bin/operator-sdk
end
Script->>FS: update_makefile_version
FS-->>Script: Makefile updated with VERSION
Script->>FS: update_csv_version
FS-->>Script: CSVs updated with OPERATOR_IMAGE and VERSION
Script->>Make: make docker-build docker-push
Note right of Make: IMG=OPERATOR_IMAGE
Make->>ContainerTool: Build and push operator image
ContainerTool-->>Make: Operator image pushed
Make-->>Script: Operator build/push complete
Script->>ContainerTool: image inspect OPERATOR_IMAGE
ContainerTool-->>Script: Metadata and size
Script->>Make: make bundle bundle-build bundle-push
Note right of Make: VERSION, IMG, BUNDLE_IMG
Make->>ContainerTool: Build and push bundle image
ContainerTool-->>Make: Bundle image pushed
Make-->>Script: Bundle build/push complete
Script->>OperatorSDK: bundle validate ./bundle
OperatorSDK-->>Script: Validation result
Script->>ContainerTool: image inspect BUNDLE_IMAGE
ContainerTool-->>Script: Metadata and size
Script-->>Developer: Print success summary and next steps
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The script relies on
grep -Pinextract_version, which is not available in BSD grep (e.g., on macOS); consider a POSIX-compatible alternative like${image##*:}orgrep -Eto improve portability. - The usage example and comment mention
build-and-push-images.shwhile the file is namedoperator_build_and_push.sh; aligning these names will reduce confusion for users invoking the script.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The script relies on `grep -P` in `extract_version`, which is not available in BSD grep (e.g., on macOS); consider a POSIX-compatible alternative like `${image##*:}` or `grep -E` to improve portability.
- The usage example and comment mention `build-and-push-images.sh` while the file is named `operator_build_and_push.sh`; aligning these names will reduce confusion for users invoking the script.
## Individual Comments
### Comment 1
<location path="scripts/operator_build_and_push.sh" line_range="118-127" />
<code_context>
+ log_info "Updating Makefile version to ${VERSION}..."
+
+ if [[ -f "Makefile" ]]; then
+ sed -i "s/^VERSION ?= .*/VERSION ?= ${VERSION}/" Makefile
+ log_success "Makefile version updated"
+ else
+ log_error "Makefile not found"
+ exit 1
+ fi
+}
+
+# Update CSV version
+update_csv_version() {
+ log_info "Updating CSV version to ${VERSION}..."
+
+ # Update bundle CSV file
+ CSV_FILE="bundle/manifests/kruize-operator.clusterserviceversion.yaml"
+ if [[ -f "$CSV_FILE" ]]; then
+ sed -i "s|containerImage: .*|containerImage: ${OPERATOR_IMAGE}|" "$CSV_FILE"
+ sed -i "s/name: kruize-operator\.v.*/name: kruize-operator.v${VERSION}/" "$CSV_FILE"
+ log_success "Bundle CSV updated"
+ else
</code_context>
<issue_to_address>
**issue (bug_risk):** Use a `sed -i` form that is portable across GNU/BSD sed
This form works on GNU sed but fails on BSD sed (e.g., macOS), which requires a backup suffix (even an empty one). Since this script may run on multiple environments, please switch to a portable form like `sed -i '' ...` or add a small `uname`-based branch to select the appropriate `sed -i` invocation.
</issue_to_address>
### Comment 2
<location path="scripts/operator_build_and_push.sh" line_range="8-11" />
<code_context>
+CONTAINER_TOOL="${CONTAINER_TOOL:-docker}"
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Clarify or align `CONTAINER_TOOL` with the hardcoded docker-based make targets
`CONTAINER_TOOL` defaults to `docker` and is used only for image inspection, while build/push still run `make docker-build docker-push`. If someone sets `CONTAINER_TOOL=podman`, they may expect the whole flow to use podman. Please either document that `CONTAINER_TOOL` only affects verification, or adjust the make targets so build/push also respect this setting.
```suggestion
# Default values
OPERATOR_IMAGE=""
BUNDLE_IMAGE=""
# Note: CONTAINER_TOOL is used only for image inspection/verification in this script.
# Build and push still rely on docker-based make targets (e.g., `make docker-build docker-push`),
# regardless of this setting.
CONTAINER_TOOL="${CONTAINER_TOOL:-docker}"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
update_csv_versionfunction usessed -iwithout the macOS/BSD guard you added inupdate_makefile_version, which will break on Darwin; consider factoring a small helper for portable in-placesedand using it consistently. - The
OSTYPEcheck inupdate_makefile_versioncompares to"Darwin", but typical values are lowercase likedarwin*; using a case pattern match (e.g.,case "$OSTYPE" in darwin*)) would make this more robust. - Since the script mutates files (Makefile, CSVs) before building, you might want to warn or prompt if there are uncommitted changes in the working tree to avoid accidentally altering a clean branch.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `update_csv_version` function uses `sed -i` without the macOS/BSD guard you added in `update_makefile_version`, which will break on Darwin; consider factoring a small helper for portable in-place `sed` and using it consistently.
- The `OSTYPE` check in `update_makefile_version` compares to `"Darwin"`, but typical values are lowercase like `darwin*`; using a case pattern match (e.g., `case "$OSTYPE" in darwin*)`) would make this more robust.
- Since the script mutates files (Makefile, CSVs) before building, you might want to warn or prompt if there are uncommitted changes in the working tree to avoid accidentally altering a clean branch.
## Individual Comments
### Comment 1
<location path="scripts/operator_build_and_push.sh" line_range="143-145" />
<code_context>
+ log_info "Updating CSV version to ${VERSION}..."
+
+ # Update bundle CSV file
+ CSV_FILE="bundle/manifests/kruize-operator.clusterserviceversion.yaml"
+ if [[ -f "$CSV_FILE" ]]; then
+ sed -i "s|containerImage: .*|containerImage: ${OPERATOR_IMAGE}|" "$CSV_FILE"
+ sed -i "s/name: kruize-operator\.v.*/name: kruize-operator.v${VERSION}/" "$CSV_FILE"
+ log_success "Bundle CSV updated"
</code_context>
<issue_to_address>
**issue (bug_risk):** Make the sed -i usage in CSV updates macOS-compatible as done in update_makefile_version.
Here `sed -i` (and in the `BASE_CSV_FILE` update) will fail on macOS’s BSD sed because it requires a backup suffix. Please reuse the existing Darwin/other `sed -i` handling from `update_makefile_version`, or factor out a small helper, so these CSV updates work consistently on both Linux and macOS.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Contributor
Author
|
Example Script output: Files to be committed after the script completes: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a script to build and push operator and bundle images and also verify the bundle image using operator-sdk cmds.
Summary by Sourcery
Add a script to automate building, pushing, and verifying Kruize operator and bundle images based on a shared version.
Enhancements:
Build: