refactor: profile-controller manifests similar to notebooks v2#199
Conversation
|
Move kubeflow overlay Istio manifests (authorization policy, virtual service, params config) into config/components/istio and compose them from overlays/kubeflow. Update common labels in default and overlay kustomizations and align the authorization policy selector with app labels. Signed-off-by: alokdangre <alokdangre@gmail.com>
Reorganize profile-controller manifests under manifests/kustomize.\nUpdate Makefile, release tooling, and install script paths to the new layout. Signed-off-by: alokdangre <alokdangre@gmail.com>
ddc831a to
f00e26b
Compare
Switch workflows to manifests/kustomize and update envtest CRD path for profile-controller unit tests. Signed-off-by: alokdangre <alokdangre@gmail.com>
12dbbfe to
311fad8
Compare
Signed-off-by: alokdangre <alokdangre@gmail.com>
andyatmiami
left a comment
There was a problem hiding this comment.
Most importantly - I appreciate your willingness to learn and jump in on this admittedly complex refactoring of profile-controller. I'm sure you and I both will learn a lot as we get this PR in shape to be merge-able.
That being said - we have a lot of work we need to do to get this PR ready 😇
I would highly encourage you to make the folder hierarchy AS SIMILAR AS POSSIBLE to what the controller directory looks like in notebooks-v2:
profile-controller as compared to notebooks-v2/controller - as profile-controller acts like both the backend and controller of notebooks-v2 as a single thing. Additionally, the kfam logic in this repo is also wild and not a pattern you will find in notebooks-v2 😎
ℹ️ I'm sure there is going to be a couple rounds of reviews necessary to get this ready to roll. As such - please understand all the PR comments contained herein are NOT exhaustive. Lets focus first on fixing, above all else, the directory structure. As other things clean up - I can then focus more on the content of the files - which I didn't have enough focus time to do on this PR.
4dacdfa to
fa661ec
Compare
…s-v2 - Flatten crd directory: remove bases/ and patches/ subdirs - Move RBAC files to base/manager/ - Create base/webhook/ for webhook patches - Create components/kfam/ for kfam service - Move auth_proxy resources to components/prometheus/ - Update kustomization.yaml files to use new structure - Replace deprecated kustomize fields (commonLabels, namePrefix, vars) - Hardcode resource names with profiles- prefix - Update Makefile and suite_test.go paths Signed-off-by: alokdangre <alokdangre@gmail.com>
fa661ec to
d360d4a
Compare
@andyatmiami thanks fo reviewing, i now understand and implemented the folder structure as per notebooks v2 |
|
/ok-to-test |
|
Thanks to everyone for working to get this over the line, especially @andyatmiami and @alokdangre for doing extended live coding sessions to figure a lot of this legacy stuff out. I have been pushing commits as we were working, and am now happy to merge this, especially as all tests now pass, and because we will be doing more testing before we cut a final 2.0.0 release anyway, hopefully we come across anything we missed during that process. But thanks again as this series of PRs has left the dashboard manifests in a much more maintainable state for future releases! /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 |
Summary
Refactor
profile-controllermanifests to match the notebooks-stylemanifests/kustomizelayout and keep Istio resources as a reusable component.config/tomanifests/kustomize/.base/components/overlays/samples/components/istio/{authorizationpolicy,virtual-service,params,kustomization}.yamloverlays/kubeflow/kustomization.yamlto compose Istio via:components: ../../components/istioservice.yamlandpatches/.base/kustomization.yamland nest generated resources under:base/crd/base/manager/base/rbac/base/patches/components/profile-controller/Makefiletesting/shared/install_profile_controller.shreleasing/update-manifests-images.pymanifests/kustomize/base/kustomization.yamlmanifests/kustomize/overlays/kubeflow/kustomization.yamlmanifests/kustomize/overlays/standalone/kustomization.yamlapp: profiles.related: #180
Directory Structure
Note: kustomize deprecation warnings (
commonLabels,patchesStrategicMerge,vars) are pre-existing and not introduced by this refactor.