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.

Sign up for GitHub

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

Jump to bottom

feat: support toleration config #1731

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: support toleration config #1731

wants to merge 1 commit into from

Conversation

tico88612
Copy link

@tico88612 tico88612 commented Apr 20, 2024 •
edited

Purpose of PR?:

Fixes #1720

Does this PR introduce a breaking change?

Probably not.

If the changes in this PR are manually verified, list down the scenarios covered::

Additional information for reviewer? :

I'm a newbie developer operator CRD. I need suggestions about coding style or any important that I missed.

BTW, I haven't written a test yet, need advice.

Checklist:

@tico88612 tico88612 force-pushed the feat/toleration branch 2 times, most recently from f95f9ca to b2c6863 Compare April 23, 2024 03:22
@rksharma95
Copy link
Collaborator

@tico88612 please rebase to the main and squash the commits.

@tico88612 tico88612 changed the title feat: KubeArmor support toleration config feat: support toleration config May 1, 2024
@tico88612
Copy link
Author

@rksharma95 rebased.
BTW I'm a newbie developer operator CRD.
I need suggestions about coding style or any important that I missed.

@rksharma95
Copy link
Collaborator

@rksharma95 rebased. BTW I'm a newbie developer operator CRD. I need suggestions about coding style or any important that I missed.

the changes looks good to me 👍 , let me know if you have any specific question i will try best to answer that.

rksharma95
rksharma95 previously approved these changes May 6, 2024
View reviewed changes
@tico88612
feat: support toleration config
652f92f 
Feat: KubeArmor chart values add kubearmor-controller toleration

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Feat: KubeArmor chart values add kubearmor-relay toleration

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Typo: KubeArmor chart values comment wrong fix

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Feat: KubeArmorOperator chart values add deployment toleration

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Chore: update KubeArmorOperator controller-gen from v0.4.1 to v0.14.0

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Feat: KubeArmorConfigSpec Relay & Controller add Toleration

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Command: controller-gen update KubeArmorConfig CRD

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Feat: add KubeArmorConfig toleration default value

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>

Feat: implement tolerations in operator

Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>
@daemon1024
Copy link
Member

@rksharma95 rksharma95 dismissed their stale review May 18, 2024 06:25

manual test failed

@rksharma95
Copy link
Collaborator

@tico88612 I tried to test the PR, seems like there are some issues, operator is not able to handle the toleration config.

kubearmorconfig 
spec:
  kubeRbacProxyImage:
    imagePullPolicy: Always
  kubearmorControllerImage:
    image: kubearmor/kubearmor-controller:latest
    imagePullPolicy: Always
  kubearmorImage:
    image: kubearmor/kubearmor:stable
    imagePullPolicy: Always
  kubearmorInitImage:
    image: kubearmor/kubearmor-init:stable
    imagePullPolicy: Always
  kubearmorRelayImage:
    image: kubearmor/kubearmor-relay-server:latest
    imagePullPolicy: Always
  kubearmorRelayToleration:
  - effect: NoSchedule
    key: arch
    operator: Equal
    value: amd64

Relay Pod:

> kubectl get pods -n kubearmor kubearmor-relay-8464877449-t6gcr -o wide
NAME                               READY   STATUS    RESTARTS   AGE   IP            NODE                                           NOMINATED NODE   READINESS GATES
kubearmor-relay-8464877449-t6gcr   1/1     Running   0          23h   10.84.3.240   gke-ai-ml-test-ka-default-pool-8482024a-1kmx   <none>           <none>

> kubectl get pod -n kubearmor kubearmor-relay-8464877449-t6gcr  -o jsonpath='{.spec.tolerations}'
[{"effect":"NoExecute","key":"node.kubernetes.io/not-ready","operator":"Exists","tolerationSeconds":300},{"effect":"NoExecute","key":"node.kubernetes.io/unreachable","operator":"Exists","tolerationSeconds":300}]

Node:

> kubectl get node gke-ai-ml-test-ka-default-pool-8482024a-1kmx -o jsonpath='{.spec.taints}'
[{"effect":"NoSchedule","key":"arch","value":"arm64"}]

@tico88612
Copy link
Author

What are some of the things I've missed about the modifications?

@rksharma95
Copy link
Collaborator

What are some of the things I've missed about the modifications?

I don't see anything missing, there might be something not working as expected.

@rksharma95
Copy link
Collaborator

@tico88612 any update here? let me know if you need any assistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rksharma95 rksharma95 rksharma95 left review comments

@DelusionalOptimist DelusionalOptimist Awaiting requested review from DelusionalOptimist

@Prateeknandle Prateeknandle Awaiting requested review from Prateeknandle

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
v1.4.0 Release
Status: In Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Request: support for taints and tolerations to KubeArmor deployments in Helm charts.
3 participants
@tico88612 @rksharma95 @daemon1024