Backport PR #2736 to release/v1.7 for update ubuntu version for devcontainer #2750
Conversation
* ♻️ update ubuntu version for devcontainer Signed-off-by: vankichi <kyukawa315@gmail.com> * 💚 add platform opts Signed-off-by: vankichi <kyukawa315@gmail.com> * ♻️ Remove unnecessary installation Signed-off-by: vankichi <kyukawa315@gmail.com> * ♻️ Change install helm command Signed-off-by: vankichi <kyukawa315@gmail.com> * ♻️ Fix Signed-off-by: vankichi <kyukawa315@gmail.com> * ♻️ Fix Signed-off-by: vankichi <kyukawa315@gmail.com> * ♻️ Fix Signed-off-by: vankichi <kyukawa315@gmail.com> * 💚 remove platform opts Signed-off-by: vankichi <kyukawa315@gmail.com> --------- Signed-off-by: vankichi <kyukawa315@gmail.com> Co-authored-by: Yusuke Kato <kpango@vdaas.org>
Deploying vald with
|
| Latest commit: |
dfdd825
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://553bc2b5.vald.pages.dev |
| Branch Preview URL: | https://backport-release-v1-7-refact-myw3.vald.pages.dev |
📝 WalkthroughWalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
hack/docker/gen/main.gois excluded by!**/gen/**
📒 Files selected for processing (2)
Makefile.d/helm.mk(2 hunks)dockers/dev/Dockerfile(2 hunks)
🔇 Additional comments (4)
dockers/dev/Dockerfile (2)
22-22: Verify tool compatibility with Ubuntu 24.04
Given this is a development container with extensive tooling, please ensure all installed components are compatible with Ubuntu 24.04:
- Verify all Make targets work correctly
- Test the complete container build process
- Document any known compatibility issues or required workarounds
#!/bin/bash
# Check for any Ubuntu version specific configurations or dependencies
rg -i "ubuntu.*?(22\.04|24\.04)" .
# Look for version-specific package dependencies in make targets
fd Makefile . -x grep -l "apt-get"Also applies to: 70-71
70-71: Verify removal of software-properties-common dependency
The package installation order change for curl and gnupg is good. However, please verify that removing software-properties-common doesn't affect any subsequent operations in the Dockerfile, particularly:
- Node.js setup that uses the nodesource repository
- Any other commands that might use
add-apt-repository
✅ Verification successful
Let me gather more specific information about the Node.js installation and repository setup in the Dockerfile.
Removal of software-properties-common package is safe
The Node.js repository setup is using the official nodesource script (curl -fsSL https://deb.nodesource.com/setup_current.x | bash -) which doesn't require software-properties-common. The Dockerfile doesn't use add-apt-repository anywhere, and all package installations are done directly through apt-get. The removal of software-properties-common package won't affect any operations in this Dockerfile.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential uses of add-apt-repository or other software-properties-common tools
rg -i "add-apt-repository|software-properties" .
# Check if any installation scripts in the repository use these commands
fd -t f ".*\.(sh|bash)$" . -x grep -l -i "add-apt-repository"
Length of output: 451
Script:
#!/bin/bash
# Check the Dockerfile for Node.js installation and repository setup
cat dockers/dev/Dockerfile
Length of output: 4795
Makefile.d/helm.mk (2)
23-28: LGTM! Improved cross-platform compatibility
The changes enhance the Helm installation process by:
- Properly handling different architectures (aarch64/arm64, x86_64/amd64)
- Using a more reliable tarball-based installation
- Improving maintainability through proper variable usage
40-40: LGTM! Standardized tar syntax
The change from tar xzvf to tar -xzvf improves consistency with GNU tar standard syntax.
| @@ -19,7 +19,7 @@ | |||
| # DO_NOT_EDIT this Dockerfile is generated by https://github.com/vdaas/vald/blob/main/hack/docker/gen/main.go | |||
| ARG UPX_OPTIONS=-9 | |||
| # skipcq: DOK-DL3026,DOK-DL3007 | |||
| FROM mcr.microsoft.com/devcontainers/base:ubuntu22.04 | |||
| FROM mcr.microsoft.com/devcontainers/base:ubuntu24.04 | |||
There was a problem hiding this comment.
Reconsider Ubuntu 24.04 upgrade in release branch
Upgrading to Ubuntu 24.04 in a release branch (v1.7) raises concerns:
- Ubuntu 24.04 is currently in development/beta phase
- This could introduce unexpected stability issues in the development environment
- Release branches typically should maintain stable, well-tested configurations
Consider keeping Ubuntu 22.04 LTS for the v1.7 release branch and potentially targeting this upgrade for the next major release instead.
|
[CHATOPS:HELP] ChatOps commands.
|
kpango
deleted the
backport/release/v1.7/refactor/hack-dockers/update-ubuntu-version-for-devcontainer
branch
Description
SSIA
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
New Features
Bug Fixes
Chores