Merge dev and ci container #3425
Conversation
📝 WalkthroughWalkthroughSwitched CI references to the dev container: replaced many GitHub Actions jobs' image from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (180)
🔇 Additional comments (3)
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. Comment |
|
[CHATOPS:HELP] ChatOps commands.
|
Deploying vald with
|
| Latest commit: |
eb9b969
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5b4eae57.vald.pages.dev |
| Branch Preview URL: | https://refactor-docker-aio-containe.vald.pages.dev |
github-actions
bot
added
type/ci
actions/backport/release/v1.7
actions/build/amd64
|
/format |
|
[FORMAT] Updating license headers and formatting go codes triggered by kmrmt. |
Signed-off-by: Vdaas CI <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3425 +/- ##
==========================================
- Coverage 20.38% 18.04% -2.35%
==========================================
Files 83 127 +44
Lines 10702 12107 +1405
==========================================
+ Hits 2182 2185 +3
- Misses 8246 9647 +1401
- Partials 274 275 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Profile Report
|
kmrmt
requested review from
a team,
Matts966 and
kpango
and removed request for
a team
![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
🧹 Nitpick comments (1)
.github/workflows/_detect-ci-container.yaml (1)
16-16: LGTM! Workflow correctly updated to detect dev container.The workflow name and TARGET_IMAGE environment variable have been appropriately updated to reference the dev container.
Optional: Consider renaming the workflow file for consistency.
The workflow file is still named
_detect-ci-container.yamlbut now detects the dev container. While this doesn't affect functionality, renaming it to_detect-dev-container.yamlwould improve clarity. Similarly, the job namedetect-ci-containerused across many workflows could be updated for consistency.📝 Suggested workflow file rename
- Rename
.github/workflows/_detect-ci-container.yaml→.github/workflows/_detect-dev-container.yaml- Update all workflow references from
uses: ./.github/workflows/_detect-ci-container.yamltouses: ./.github/workflows/_detect-dev-container.yaml- Optionally rename job references from
detect-ci-containertodetect-dev-containerAlso applies to: 24-24
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
hack/docker/gen/main.gois excluded by!**/gen/**hack/docker/gen/main_test.gois excluded by!**/gen/**
📒 Files selected for processing (32)
.gitfiles.github/actions/detect-docker-image-tags/action.yaml.github/actions/setup-helm/action.yaml.github/workflows/_detect-ci-container.yaml.github/workflows/_release-pr.yaml.github/workflows/build-binaries.yaml.github/workflows/build-protobuf.yaml.github/workflows/chatops.yaml.github/workflows/codeql-analysis.yml.github/workflows/coverage.yaml.github/workflows/dockers-ci-container-image.yaml.github/workflows/e2e-chaos.yaml.github/workflows/e2e-code-bench-agent.yaml.github/workflows/e2e-max-dim.yaml.github/workflows/e2e-profiling.yaml.github/workflows/e2e.v2.yaml.github/workflows/e2e.yaml.github/workflows/format.yaml.github/workflows/fossa.yaml.github/workflows/helm-lint.yaml.github/workflows/helm.yaml.github/workflows/reviewdog-k8s.yaml.github/workflows/reviewdog.yaml.github/workflows/test-hack.yaml.github/workflows/unit-test-rust.yaml.github/workflows/unit-test.yaml.github/workflows/update-deps.yamlMakefileMakefile.d/docker.mkdockers/ci/base/Dockerfiledockers/ci/base/README.mddockers/dev/Dockerfile
💤 Files with no reviewable changes (5)
- Makefile
- dockers/ci/base/README.md
- .github/workflows/dockers-ci-container-image.yaml
- Makefile.d/docker.mk
- dockers/ci/base/Dockerfile
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Matts966
Repo: vdaas/vald PR: 3105
File: .github/workflows/e2e.v2.yaml:108-112
Timestamp: 2025-10-14T07:12:18.284Z
Learning: In the vdaas/vald repository, the CI container image `ghcr.io/vdaas/vald/vald-ci-container` has Docker CLI available and can successfully execute `docker system df` and `docker system prune` commands within GitHub Actions containerized jobs, despite running inside a container itself.
Learnt from: Matts966
Repo: vdaas/vald PR: 2928
File: pkg/gateway/lb/handler/grpc/linear_search.go:271-333
Timestamp: 2025-04-15T05:32:26.428Z
Learning: For the vald project, refactoring PRs should be kept separate from semantic changes that alter behavior. The current PR "Divide LB Gateway handler.go" is focused solely on splitting the file into smaller, more manageable components without changing functionality.
📚 Learning: 2025-10-14T07:12:18.284Z
Learnt from: Matts966
Repo: vdaas/vald PR: 3105
File: .github/workflows/e2e.v2.yaml:108-112
Timestamp: 2025-10-14T07:12:18.284Z
Learning: In the vdaas/vald repository, the CI container image `ghcr.io/vdaas/vald/vald-ci-container` has Docker CLI available and can successfully execute `docker system df` and `docker system prune` commands within GitHub Actions containerized jobs, despite running inside a container itself.
Applied to files:
.github/workflows/build-binaries.yaml.github/workflows/fossa.yaml.github/workflows/helm.yaml.github/workflows/test-hack.yaml.github/workflows/e2e-max-dim.yaml.gitfiles.github/workflows/coverage.yaml.github/workflows/unit-test-rust.yaml.github/workflows/unit-test.yaml.github/workflows/e2e-profiling.yaml.github/workflows/_release-pr.yaml.github/workflows/chatops.yaml.github/workflows/reviewdog.yaml.github/workflows/e2e.v2.yaml.github/workflows/helm-lint.yaml.github/workflows/_detect-ci-container.yaml.github/workflows/format.yaml.github/workflows/reviewdog-k8s.yaml.github/workflows/build-protobuf.yaml.github/actions/detect-docker-image-tags/action.yaml.github/workflows/e2e-chaos.yaml.github/workflows/e2e-code-bench-agent.yaml.github/workflows/codeql-analysis.yml.github/workflows/e2e.yaml.github/workflows/update-deps.yaml
📚 Learning: 2025-10-01T09:51:47.956Z
Learnt from: acrossd-matsui
Repo: vdaas/vald PR: 3213
File: .github/workflows/dockers-dev-container-image.yaml:69-75
Timestamp: 2025-10-01T09:51:47.956Z
Learning: In the vdaas/vald repository, workflows using pull_request_target with secrets are protected by a ci/approved label check in the reusable _docker-image.yaml workflow, which ensures fork PRs only access secrets after manual approval.
Applied to files:
.github/workflows/build-binaries.yaml.github/workflows/fossa.yaml.github/workflows/helm.yaml.github/workflows/test-hack.yaml.github/workflows/e2e-max-dim.yaml.gitfiles.github/workflows/coverage.yaml.github/workflows/unit-test-rust.yaml.github/workflows/unit-test.yaml.github/workflows/e2e-profiling.yaml.github/workflows/_release-pr.yaml.github/workflows/chatops.yaml.github/workflows/reviewdog.yaml.github/workflows/helm-lint.yaml.github/workflows/_detect-ci-container.yaml.github/workflows/format.yaml.github/workflows/reviewdog-k8s.yaml.github/workflows/build-protobuf.yaml.github/workflows/e2e-chaos.yaml.github/workflows/e2e-code-bench-agent.yaml.github/workflows/codeql-analysis.yml.github/workflows/e2e.yaml.github/workflows/update-deps.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (146)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: coverage
- GitHub Check: runner / go build
- GitHub Check: CodeQL Analyze (rust)
- GitHub Check: CodeQL Analyze (go)
- GitHub Check: Run tests for hack packages
- GitHub Check: Run tests for internal packages
- GitHub Check: Run tests for cmd packages
- GitHub Check: Run tests for pkg packages
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
- GitHub Check: build / build (amd64, ubuntu-latest)
🔇 Additional comments (27)
.github/workflows/codeql-analysis.yml (1)
77-77: The container image change is correct—no verification needed.The
vald-dev-containerincludes all tools required for CodeQL analysis: Go compiler and build tools (viamake go/installin Dockerfile), Rust toolchain (viamake RUST_VERSION rust/install), Docker CLI (viamake docker-cli/install), and comprehensive development dependencies (build-essential, gcc, g++, cmake, git, etc.). The switch fromvald-ci-containeris consistent with the PR's goal to merge the two containers and does not introduce any tooling gaps for the CodeQL job's autobuild on Go or Rust analysis..github/workflows/fossa.yaml (1)
49-49: LGTM! Container image consolidation is correct.The switch from
vald-ci-containertovald-dev-containeris complete. Thevald-dev-containerimage includes all required tools for this workflow:curl,git,jq, andbash, plus Docker CLI support maintained from the CI container..github/workflows/_release-pr.yaml (1)
64-64: All required tools are available in vald-dev-container. The migration fromvald-ci-containertovald-dev-containeris complete and correct. The dev container includes git, make, curl, jq, sed, and gpg—all tools required by this release job. The detection workflow is already updated to target the dev container..gitfiles (3)
87-88: LGTM: New aio container workflow files added.The addition of
dockers-aio-container-image.yamlanddockers-aio-image.yamlworkflows aligns with the PR objective to create a merged all-in-one container. The naming convention is consistent with existing workflow files.
620-621: LGTM: New aio container Dockerfile and documentation added.The addition of
dockers/aio/Dockerfileanddockers/aio/README.mdproperly establishes the infrastructure for the new all-in-one container. The file organization follows the existing pattern used by other container variants.
87-88: Dev container workflow is still in use and not migrated to aio.The
dockers-dev-container-image.yamlworkflow still exists and is actively referenced across multiple workflows (unit-test.yaml, reviewdog.yaml, helm.yaml, etc.), yet there are no references to an aio container in any workflow files. The PR objectives state the CI and dev containers should be merged into a single aio container, but the dev container migration appears incomplete. Either the dev-container workflow and all its usages need to be updated to reference the new aio container, or the .gitfiles additions for aio workflows (lines 87-88) should not be included until the migration is fully completed.dockers/dev/Dockerfile (1)
75-75: LGTM! Addition of sudo supports the container merge.The addition of the
sudopackage is appropriate for a unified dev/CI container, as CI workflows often require elevated privileges for certain operations..github/workflows/build-binaries.yaml (1)
43-43: LGTM! Container image reference updated consistently.The switch from
vald-ci-containertovald-dev-containeraligns with the PR's objective to consolidate containers. The tag resolution mechanism remains unchanged..github/workflows/format.yaml (1)
48-48: LGTM! Container references updated consistently across both jobs.Both the
formatandcheck-format-diffjobs now use the unifiedvald-dev-container:nightlyimage, maintaining consistency with the container consolidation effort.Also applies to: 121-121
.github/workflows/update-deps.yaml (1)
43-43: LGTM! Dependency update workflow migrated to unified container.The
update-versionjob now uses the mergedvald-dev-containerimage with the same tag resolution logic..github/workflows/coverage.yaml (1)
55-55: LGTM! Coverage workflow updated to use unified container.The coverage job's container reference has been successfully migrated to
vald-dev-container..github/workflows/reviewdog.yaml (1)
45-45: LGTM! Reviewdog workflows migrated to unified container.Both
golangci-lintandgo-buildjobs now reference the consolidatedvald-dev-containerimage. Based on learnings, the dev container includes the necessary tooling for these operations.Also applies to: 72-72
.github/workflows/chatops.yaml (1)
61-61: LGTM! All ChatOps jobs migrated to unified container.All five ChatOps jobs (
label,rebase,gentest,format,approve) consistently reference the mergedvald-dev-container:nightlyimage, maintaining uniformity across the workflow.Also applies to: 114-114, 190-190, 300-300, 428-428
.github/workflows/unit-test.yaml (1)
63-63: LGTM! All unit test jobs migrated to unified container.The three test jobs (
test-cmd,test-internal,test-pkg) consistently reference the consolidatedvald-dev-containerimage with appropriate tag resolution.Also applies to: 88-88, 113-113
.github/workflows/reviewdog-k8s.yaml (1)
55-55: LGTM! Consistent container image migration.All three jobs (kubeval-conflint, kubelinter-k8s-manifests, kubelinter-vald-chart) consistently updated to use the dev container image while preserving the same tag reference mechanism.
Also applies to: 83-83, 110-110
.github/workflows/e2e-code-bench-agent.yaml (1)
70-70: LGTM! Consistent image updates for benchmark jobs.Both benchmark jobs (grpc-sequential and grpc-stream) consistently updated to use the dev container image.
Also applies to: 105-105
.github/actions/detect-docker-image-tags/action.yaml (1)
55-55: LGTM! Container mapping correctly updated.The associative array entry has been appropriately updated to reflect the container name change from CI container to dev container.
.github/workflows/build-protobuf.yaml (1)
52-52: LGTM! Build job correctly updated.Container image reference updated to use the dev container.
.github/workflows/unit-test-rust.yaml (1)
53-53: LGTM! Test job correctly updated.Container image reference updated to use the dev container for Rust testing.
.github/workflows/helm-lint.yaml (1)
52-52: LGTM! Helm lint jobs consistently updated.All three Helm lint jobs (lint-vald-chart, lint-vald-helm-operator-chart, lint-values-schema) uniformly updated to use the dev container image.
Also applies to: 78-78, 104-104
.github/workflows/e2e-chaos.yaml (1)
55-55: Dev container already includes all necessary E2E testing tools.Verification confirms the dev container Dockerfile (./dockers/dev/Dockerfile) explicitly installs docker-cli, kubectl, helm, k3d, kind, make, git, jq, and other required tools for E2E testing. No additional tools are needed.
.github/workflows/e2e.v2.yaml (1)
126-126: LGTM! Consistent container image update.The change is consistent with the container consolidation effort. The verification requested in the previous workflow applies here as well.
.github/workflows/test-hack.yaml (1)
67-67: LGTM! Container image updated correctly.The update maintains consistency with the container consolidation changes across the repository.
.github/workflows/helm.yaml (1)
47-47: LGTM! Both Helm workflow jobs updated consistently.Both the
update-helm-chartandcrud-on-remote-helm-chartjobs have been updated to use the consolidated dev container, maintaining consistency across the workflow.Also applies to: 95-95
.github/workflows/e2e.yaml (1)
53-53: LGTM! All E2E jobs updated consistently.All eight E2E job variants in this workflow have been updated to use the consolidated dev container. The changes are uniform and maintain the same tag resolution mechanism.
Also applies to: 101-101, 172-172, 224-224, 272-272, 316-316, 357-357, 415-415
.github/workflows/e2e-profiling.yaml (1)
53-53: LGTM! Profiling workflow updated consistently.The e2e-profiling job correctly references the consolidated dev container, completing the consistent migration across all reviewed workflows.
.github/workflows/e2e-max-dim.yaml (1)
51-51: LGTM! Container image updated consistently.The switch from
vald-ci-containertovald-dev-containeraligns with the PR objectives to merge the two containers. The dev container Dockerfile explicitly installsdocker-cliandsudoviamake docker-cli/installand apt packages, ensuring all necessary tools for containerized workflows are available. The tag source is correctly preserved.
kmrmt
added
actions/build/arm64
|
/format |
|
[FORMAT] Updating license headers and formatting go codes triggered by kmrmt. |
Signed-off-by: Vdaas CI <[email protected]>
Description
vald-dev-containerandvald-ci-containerRelated Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.