-
Notifications
You must be signed in to change notification settings - Fork 74
refactor: manifests for controller #747
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
Merged
google-oss-prow
merged 2 commits into
kubeflow:notebooks-v2
from
andyatmiami:refactor/controller-kustomize
Dec 11, 2025
Merged
refactor: manifests for controller #747
google-oss-prow
merged 2 commits into
kubeflow:notebooks-v2
from
andyatmiami:refactor/controller-kustomize
Dec 11, 2025
+414
−357
Conversation
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
Contributor
Author
|
/ok-to-test |
175f162 to
f25eb56
Compare
Motivation:
This refactoring aligns the controller's kustomize configuration with best
practices and ensures consistency with other components (backend, frontend).
The primary goals were:
- Remove nested subdirectories (bases/, patches/) to match the flat structure
used in other base directories
- Ensure components are only included in overlays, not the base
- Improve maintainability through consistent naming conventions
- Better organize resources by their purpose and dependencies
Changes Made:
1. Flattened CRD directory structure:
- Moved CRD base files from crd/bases/ to crd/
- Moved patch files from crd/patches/ to crd/
- Updated all references in Makefile and test files
2. Standardized patch file naming:
- Renamed cainjection_in_*.yaml → *_cainjection_patch.yaml
- Renamed webhook_in_*.yaml → *_webhook_patch.yaml
- Provides consistent naming pattern across all patches
3. Component organization:
- Moved metrics_service.yaml to prometheus component (only included when
prometheus is enabled)
- Moved manager_webhook_patch.yaml to certmanager component (only needed
when cert-manager is enabled)
- Extracted namespace.yaml from manager.yaml for explicit definition
4. Updated references:
- Makefile: Changed controller-gen output path from crd/bases to crd
- Test files: Updated CRDDirectoryPaths in controller, webhook, and backend
test suites
5. Removed unused build-installer target:
- Removed from Makefile and README as it's not used and kustomize provides
better facilities for vendoring
Expected Manifest Output Differences:
1. webhook-service port now has name attribute:
- Added `name: webhook` to the port definition
- This enables replacements to reference the port by name, improving
maintainability and keeping port values in sync between Service and
Deployment
2. workspaces-controller-metrics-service is missing:
- This is expected as metrics_service.yaml was moved to the prometheus
component
- The service will only appear when the prometheus component is explicitly
enabled in an overlay
3. kubeflow-workspaces-istio-config ConfigMap labels:
- Common labels were added (app.kubernetes.io/managed-by, app.kubernetes.io/name,
app.kubernetes.io/part-of) due to reordering of components in the overlay
- The common component is now applied last to ensure its labels are applied
to all resources consistently
4. All other resources remain functionally equivalent:
- CRDs, Deployments, Services, RBAC, and webhook configurations are
unchanged in functionality
- Only structural and organizational improvements were made
Signed-off-by: Andy Stoneberg <[email protected]>
f25eb56 to
0fa87a6
Compare
Signed-off-by: Mathew Wicks <[email protected]> Signed-off-by: Andy Stoneberg <[email protected]>
eefe981 to
00d82c7
Compare
Member
|
Thanks! /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thesuperzapper The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
eac6059
into
kubeflow:notebooks-v2
17 of 18 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
approved
area/backend
area - related to backend components
area/controller
area - related to controller components
area/frontend
area - related to frontend components
area/v2
area - version - kubeflow notebooks v2
lgtm
ok-to-test
size/XL
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.
ℹ️ NO GH ISSUE
Motivation:
This refactoring aligns the controller's kustomize configuration with best practices and ensures consistency with other components (backend, frontend). The primary goals were:
Changes Made:
Flattened CRD directory structure:
Standardized patch file naming:
Component organization:
Updated references:
Removed unused build-installer target:
Expected Manifest Output Differences:
webhook-service port now has name attribute:
name: webhookto the port definitionworkspaces-controller-metrics-service is missing:
kubeflow-workspaces-istio-config ConfigMap labels:
All other resources remain functionally equivalent: