Skip to content
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

🌱 OCM Kueue Admission Check Controller #601

Conversation

z1ens
Copy link
Contributor

@z1ens z1ens commented Aug 19, 2024

  • Add the document for setting up Multikueue environment using OCM Kueue Admission Check Controller.
  • Add files for the environment set up.
  • Document still needs refinement.

Update 25.08

  • Fix docu.

REF: #369

kubectl create -f env/multicluster.x-k8s.io_clusterprofiles.yaml

echo "Install managed-serviceaccount"
cd /path/to/managed-serviceaccount # TODO: Replace here with your actual path.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace with git clone git@github.com:open-cluster-management-io/managed-serviceaccount.git

kubectl apply -f env/mg-sa-cma-0.6.0.yaml || true

echo "Install cluster-permission"
cd /path/to/OCM/cluster-permission # TODO: Replace here with your actual path.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace with git clone git@github.com:open-cluster-management-io/cluster-permission.git


echo "Install resource-usage-collect-addon"
cd /path/to/addon-contrib/resource-usage-collect-addon # TODO: Replace here with your actual path.
IMAGE_NAME=zheshen/resource-usage-collect-addon:latest make deploy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the image name is hardcoded in addontemplate https://github.com/open-cluster-management-io/addon-contrib/blob/main/resource-usage-collect-addon/deploy/resources/addon-template.yaml#L53 , need to replace it with example-addon-image.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes a new PR has been created to fix the image problem.
open-cluster-management-io/addon-contrib#21

apiVersion: cluster.open-cluster-management.io/v1beta1
kind: Placement
metadata:
name: placement-sample2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is inconsistent with placement-demo2-2.yaml resource name.

weight: 1
```
- You can manually edit the GPU resources on the managed clusters for testing.
```bash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some details on what user should add to their node.

parameters:
apiGroup: cluster.open-cluster-management.io
kind: Placement
name: placement-sample1 # An example placement to select clusters labeled with "nvidia-tesla-t4" GPU accelerator.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the placement name, should be placement-demo2.

@@ -0,0 +1,66 @@
apiVersion: multicluster.x-k8s.io/v1alpha1
kind: AuthTokenRequest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not think we should have this, since this is not decided yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, let's replace it with clusterpermission and managedserviceaccount.

@z1ens z1ens force-pushed the kueue-admission-check branch 2 times, most recently from fd2a5b3 to 6a37b63 Compare August 30, 2024 10:07
@z1ens z1ens changed the title [WIP] OCM Kueue Admission Check Controller 🌱 OCM Kueue Admission Check Controller Aug 30, 2024
Signed-off-by: Zhe Shen <xxtale02591@gmail.com>
@z1ens z1ens force-pushed the kueue-admission-check branch from cd44a81 to 52b8963 Compare August 30, 2024 18:56
"resources": ["authtokenrequests/status"],
"verbs": ["update", "patch"]
}
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the permission of authtokenrequests and authtokenrequests/status.

```bash
kubectl create -f ./job-demo1.yaml
```
- Check the workload on the managed clusters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the expected output for the "Check" would be nice.

Signed-off-by: Zhe Shen <xxtale02591@gmail.com>
@z1ens z1ens force-pushed the kueue-admission-check branch from f46cd9e to cf1612a Compare August 31, 2024 23:07
@haoqing0110
Copy link
Member

/lgtm

@haoqing0110
Copy link
Member

@qiujian16 could you help take a final review?

@qiujian16
Copy link
Member

I think we should add some TODO and then we can merge

  1. where to run admission check code
  2. how to enable this in ocm.

/approve
/hold

Copy link
Contributor

openshift-ci bot commented Sep 2, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, z1ens

The full list of commands accepted by this bot can be found here.

The pull request process is described here

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

@openshift-ci openshift-ci bot added the approved label Sep 2, 2024
Signed-off-by: Zhe Shen <xxtale02591@gmail.com>
@openshift-ci openshift-ci bot removed the lgtm label Sep 2, 2024
@qiujian16
Copy link
Member

/lgtm

@haoqing0110
Copy link
Member

/lgtm

@z1ens
Copy link
Contributor Author

z1ens commented Sep 2, 2024

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit 3e3d4ea into open-cluster-management-io:main Sep 2, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants