Skip to content

Conversation

@andyatmiami
Copy link
Contributor

ℹ️ 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:

  • 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

@github-project-automation github-project-automation bot moved this to Needs Triage in Kubeflow Notebooks Nov 14, 2025
@google-oss-prow google-oss-prow bot added the area/backend area - related to backend components label Nov 14, 2025
@google-oss-prow google-oss-prow bot added area/controller area - related to controller components area/v2 area - version - kubeflow notebooks v2 size/XL labels Nov 14, 2025
@andyatmiami
Copy link
Contributor Author

/ok-to-test

@andyatmiami andyatmiami force-pushed the refactor/controller-kustomize branch from 175f162 to f25eb56 Compare November 22, 2025 03:36
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 <astonebe@redhat.com>
@andyatmiami andyatmiami force-pushed the refactor/controller-kustomize branch from f25eb56 to 0fa87a6 Compare December 11, 2025 17:49
@google-oss-prow google-oss-prow bot added the area/frontend area - related to frontend components label Dec 11, 2025
@thesuperzapper thesuperzapper changed the title chore: standardize controller kustomize implementation refactor: manifests for controller Dec 11, 2025
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
@andyatmiami andyatmiami force-pushed the refactor/controller-kustomize branch from eefe981 to 00d82c7 Compare December 11, 2025 19:54
@thesuperzapper
Copy link
Member

Thanks!

/lgtm
/approve

@google-oss-prow
Copy link

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit eac6059 into kubeflow:notebooks-v2 Dec 11, 2025
17 of 18 checks passed
@github-project-automation github-project-automation bot moved this from Needs Triage to Done in Kubeflow Notebooks Dec 11, 2025
@thesuperzapper thesuperzapper deleted the refactor/controller-kustomize branch December 11, 2025 20:55
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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants