-
Notifications
You must be signed in to change notification settings - Fork 9
Virtual cluster policy #47
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
Virtual cluster policy #47
Conversation
eva-vashkevich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is Part 1. More coming...
- If I install K3k and then uninstall it, and remove the repo, the Landing page doesn't reappear for me.
- Should the "Add the k3k repository" be renamed to "Install K3K" since that's what we are doing?
3. We should probably auto-select None
https://github.com/user-attachments/assets/3b2a8171-9315-4163-a1fa-1285644287dd
4. K3KPolicyAssignmentFormatter is giving a couple warnings in the console
5. We need to verify expected Virtual cluster policy edit behavior. If I create a cluster with policy, then go to edit and change it to None, then select "virtual" virtual cluster mode, an error is displayed
pkg/virtual-clusters/tsconfig.json
Outdated
| ], | ||
| "exclude": [ | ||
| "../../node_modules" | ||
| // "../../node_modules", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we just remove the comment?
| raw | ||
| /> | ||
| <ProgressBar :progress="progress" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is giving "runtime-core.esm-bundler.js:51 [Vue warn]: Invalid prop: type check failed for prop "progress". Expected String with value "100", got Number with value 100. " warning in console
| @@ -0,0 +1,253 @@ | |||
| <script> | |||
| import { _CREATE, _EDIT } from '@shell/config/query-params'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_EDIT not used
| import LabeledSelect from '@shell/components/form/LabeledSelect'; | ||
| import { ANNOTATIONS, K3K } from '../../types'; | ||
| import { mapGetters } from 'vuex'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used
pkg/virtual-clusters/components/CruK3KCluster/ClusterPolicy.vue
Outdated
Show resolved
Hide resolved
| }, []); | ||
| }, | ||
| policy() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used
| return [this.t('generic.none'), ...this.policies.map((p) => p?.metadata?.name)]; | ||
| }, | ||
| policyAnnotation() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used
| name="k3k-cluster-mode" | ||
| :row="true" | ||
| :mode="mode" | ||
| label-key="k3k.mode.label" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we overwrite it with slot?
|
|
||
|
|
||
| virtualType({ | ||
| label: 'Virtual Clusters', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be translated?
eva-vashkevich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We should remove policies that don't have any ns from the selection and indicate that ns is required on cluster creation
- When creating VC Policy, I get runtime-core.esm-bundler.js:51 [Vue warn]: Property "cancel" was accessed during render but is not defined on instance.
at <CRUClusterPolicy key=3 ref="comp" value=
and then "[Vue warn]: Invalid prop: type check failed for prop "value". Expected Object, got String with value "". " when I click 'cancel' - Virtual Cluster Mode should be uneditable when editing existing Policy
pkg/virtual-clusters/components/CruK3KCluster/ClusterPolicy.vue
Outdated
Show resolved
Hide resolved
pkg/virtual-clusters/components/CruK3KCluster/ClusterPolicy.vue
Outdated
Show resolved
Hide resolved
| class="col span-6" | ||
| > | ||
| <LabeledSelect | ||
| :value="policy && !isEmpty(policy) ? policy : t('generic.none')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't value be null instead of t('generic.none') ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is making the LabeledSelect component display "None" when the value of policy in ClusterPolicy.vue is null
| updateName({ name }) { | ||
| this.k3kCluster.metadata.name = name; | ||
| this.k3kCluster.metadata.namespace = `k3k-${ name }`; | ||
| this.value.metadata.name = name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't setter on localValue do it already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right; good catch
pkg/virtual-clusters/promptRemove/k3k.io.virtualclusterpolicy.vue
Outdated
Show resolved
Hide resolved
| import Shortened from '@shell/components/formatter/Shortened'; | ||
| import { MANAGEMENT } from '@shell/config/types'; | ||
| const MAX_CHARS = 30; // maximum characters to show before using + n more and a tooltip to show the full list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the code so the comment is technically correct
Done
I've wired in cancel, which is defined in the CreateEditView mixin, but still seeing a warning. Button seems to work tho
Done I fixed a couple other things:
|
eva-vashkevich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one major issue
| async fetch() { | ||
| for (const policy of this.value) { | ||
| try { | ||
| const assigned = await policy.fetchAssignedClusterCount(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would Promise.all be better here?
pkg/virtual-clusters/pages/index.vue
Outdated
| async fetch() { | ||
| try { | ||
| const k3kClusterSchema = await this.$store.dispatch('cluster/find', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be done in parallel too
| const toAdd = storeObject?.nameDisplay || p; | ||
| if (maxLength && out.length + toAdd.length >= maxLength) { | ||
| out += ` ${ this.t('k3k.policy.listView.plusMore', { n: projectIds.length - i }) }`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the first toAdd is over 35 chars, we will just show plus more message. is this acceptable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }, | ||
| async fetch() { | ||
| this.fetchPolicies(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do not need to await for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, fair point. We don't really need to await this in create mode; the component wont break if it tries to render without policies loaded, and the dropdowns have their own load spinner independent of $fetchState.pending. But if not in create mode, findSelectedPolicy is likely going to call fetchPolicies over again (and await it).
pkg/virtual-clusters/edit/k3k.io.virtualclusterpolicy/Projects.vue
Outdated
Show resolved
Hide resolved
pkg/virtual-clusters/edit/k3k.io.virtualclusterpolicy/Projects.vue
Outdated
Show resolved
Hide resolved
| this.$emit('update:policy', {}); | ||
| this.$emit('update:targetNamespace', ''); | ||
| if (neu) { | ||
| this.fetchPolicies(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here with await, sorry, I thought I mentioned it in the previous review. It's okay if this intentionally doesn't wait
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is intentional
eva-vashkevich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm



#17
Description
This PR adds UI for the virtual cluster policy feature. It consists of new creation and list views for virtual cluster policy custom resource; new inputs containing policies and associated namespaces in the cluster creation form; and a landing page shown in the cluster explorer when users don't have k3k installed.
Most of the code in this PR is related to 'assigning' virtual cluster policies to projects: adding annotations to each namespace in the project.
Testing
You will want to test the namespace assignment functionality using some projects that collectively have over 200 namespaces. You can use the following yaml to add up to 1000 namespaces to your downstream cluster - update the project annotation with the ID of a project in your cluster to create the namespaces in projects.
1k namespace yaml
Landing Page
This PR addings a landing page. To the cluster explorer. The landing page should only be shown when k3k isn't installed; otherwise, clicking the 'Virtual Clusters' nav item header should redirect users to the policy list view. There's a bug in the installation button component imported from the dashboard, though it doesn't appear to effect functionality rancher/dashboard#15671
The button on the landing page should install the latest version of k3k.
Landing page screenshot
Creating Virtual Cluster Policies
This PR includes a new form to create and edit virtual cluster policies. The fields in the form are defined in the rfd- https://github.com/SUSE/rancher-architecture/pull/9/files
screenshots
Assigning Virtual Cluster Policies to Projects
Most of the code in this PR relates to the policy assignment mechanism. Users are given a multi-select of projects, and when they click save, the UI will add an annotation to each project. The UI will also add an annotation to every namespace in the project: that's the part the k3k controller actually looks at. If editing a lot of namespaces this can be slow, so the UI uses a modal force users to stay on the page while editing namespaces.
assignment screenshots + videos
https://github.com/user-attachments/assets/bef9ff14-7662-4f90-9bd8-3a99d350fbcbhttps://github.com/user-attachments/ass

ets/e2a49bc0-cb3d-4313-bddf-8e0662a5ae1c
Using policies
The k3k cluster creation form has been expanded to include a policy and namespace dropdown. The policy is not explicitly set in the k3k cluster resource's spec. Selecting a policy from the dropdown merely filters the list of available namespaces.
The policy contents should update when a new host cluster is selected. When projects are 'assigned' or 'unassigned' from a policy in the virtual cluster policy creation UI, their namespaces should be added/removed from the list shown in the cluster creation page.
When users select a namespace during cluster creation, the k3k cluster, import job, and import configmap should all be created in that namespace in the targeted host cluster.
k3k cluster creation
Screen.Recording.2025-10-20.at.8.19.53.AM.mov