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

Supports PodSecurityContext and Gatling runner container securityContext #119

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

kane8n
Copy link
Contributor

@kane8n kane8n commented May 31, 2024

Description

Provide a description of what has been changed

Checklist

Please check if applicable

  • Tests have been added (if applicable, ie. when operator codes are added or modified)
  • Relevant docs have been added or modified (if applicable, ie. when new features are added or current features are modified)

Relevant issue

#117

operation check

created user and group in the Dockerfile of the sample scenario and confirmed that it works by setting the user/group created in the securityContext.

@annappropriate
Copy link

This is great, I would love to be able to specify the sysctls for the port range!

@kane8n kane8n changed the title [WIP] Supports PodSecurityContext and Gatling runner container securityContext Supports PodSecurityContext and Gatling runner container securityContext Jun 12, 2024
@kane8n kane8n requested review from a team June 12, 2024 00:39
Copy link
Contributor

@gold-kou gold-kou left a comment

Choose a reason for hiding this comment

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

Thanks for your PR.
Just nits.

- name: net.ipv4.ip_local_port_range
value: "1024 65535"
runnerContainerSecurityContext:
runAsUser: 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this command is not available for a genaral user(1000).
So root(0) is suitable in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gold-kou
This configuration is a sample to show that SecurityContext works, and the user/group specified here is created in the Dockerfile to make the sample work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is just a sample, however even if sample, I just thought the value should be appropriate one.
However, as u said this sample works as expected. I checked the value in a gatling runner container.

gatling@gatling-sample01-runner-4ml82:/opt/gatling$ sysctl net.ipv4.ip_local_port_range
net.ipv4.ip_local_port_range = 1024	65535

Looks cool.

@@ -8,6 +8,13 @@ spec:
notifyReport: false # The flag of notifying gatling report
cleanupAfterJobDone: true # The flag of cleaning up gatling jobs resources after the job done
podSpec:
securityContext:
sysctls:
Copy link
Contributor

Choose a reason for hiding this comment

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

memo

Sysctls is only available in PodSecurityContext.

docs/api.md Outdated
@@ -136,6 +134,8 @@ _Appears in:_
| `tolerations` _[Toleration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#toleration-v1-core) array_ | (Optional) Tolerations specification. |
| `serviceAccountName` _string_ | (Required) ServiceAccountName specification. |
| `volumes` _[Volume](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#volume-v1-core) array_ | (Optional) volumes specification. |
| `securityContext` _[PodSecurityContext](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#podsecuritycontext-v1-core)_ | (Optional) SecurityContext specification. |
| `runnerContainerSecurityContext` _[SecurityContext](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#securitycontext-v1-core)_ | (Optional) RunecontainerSecurityContext specifies the SecurityContext of the Gatling runner container. |
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

@kane8n kane8n requested a review from gold-kou June 17, 2024 03:07
Copy link
Contributor

@gold-kou gold-kou left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@itiB itiB left a comment

Choose a reason for hiding this comment

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

I've added procps to gatling/Dockerfile in order to use the ps command and entered the Pod to confirm that the settings are being applied correctly. I was able to verify that both sysctls and port_range are set, so the code you've written seems to be working well! Excellent job.

Checked point

Test simulation manifest

apiVersion: gatling-operator.tech.zozo.com/v1alpha1
kind: Gatling
metadata:
  name: gatling-sample01
spec:
  generateReport: false
  cleanupAfterJobDone: true
  podSpec:
    securityContext:
      runAsUser: 0
      runAsGroup: 0
      sysctls:
        - name: net.ipv4.ip_local_port_range
          value: "1024 65535"
    runnerContainerSecurityContext:
      runAsUser: 1000
      runAsGroup: 1000
    serviceAccountName: "gatling-operator-worker"
    gatlingImage: ghcr.io/st-tech/gatling:latest
  testScenarioSpec:
    parallelism: 3
    simulationClass: "MyBasicSimulation"
    env:
      - name: ENV
        value: "dev"
      - name: CONCURRENCY
        value: "1"
      - name: DURATION
        value: "60"

Detail about created pod

❯ make kind-sample-deploy
Cluster already exists
cd gatling && docker build -t gatling:20240618-120429 .
...
gatling.gatling-operator.tech.zozo.com/gatling-sample01 created

support-security-context ❯ k exec -it gatling-sample01-runner-6b6t2 -- /bin/sh
Defaulted container "gatling-runner" out of: gatling-runner, gatling-waiter (init)
$ ps -eo pid,user,uid,comm
    PID USER       UID COMMAND
      1 gatling   1000 sh
     15 gatling   1000 gatling.sh
     19 gatling   1000 java
     85 gatling   1000 java
    127 gatling   1000 sh
    142 gatling   1000 ps
$ sysctl net.ipv4.ip_local_port_range
net.ipv4.ip_local_port_range = 1024     65535
$ 

@itiB itiB requested a review from a team June 18, 2024 03:12
@kane8n kane8n merged commit 215d73b into main Jun 27, 2024
3 checks passed
@kane8n kane8n deleted the support-security-context branch June 27, 2024 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants