-
Notifications
You must be signed in to change notification settings - Fork 20
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
Move away from Jobs to Pods #266
base: main
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lpiwowar 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 |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/9f2d32ae798a498fb41c1b99fcb8cb1e ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 22m 41s |
With this PR [1] the test operator dropped the usage of OCP Jobs for spawning of the test pods. This change updates the test-operator role so that it works with the new version of the test-operator that spawns test pods directly through the OCP Pods object. [1] openstack-k8s-operators/test-operator#266
cdb82e1
to
9efbbb1
Compare
This change depends on a change that failed to merge. Change openstack-k8s-operators/ci-framework#2604 is needed. |
recheck |
In this PR [1] we dropped the usage of the Jobs by the test-operator. This allows us to drop the rights for: - ServiceAccount managing & creation - Role managing & creation - RoleBinding managing & creation These rights were only needed because Jobs were required to be spawned with an extra ServiceAccount that would have elevated privileges in case the test pods need to run with privileged SecurityContext. [1] openstack-k8s-operators#266 Depends-On: openstack-k8s-operators#266
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/8d8a9fbde32c44ddbc4991307290d9e9 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 15m 41s |
With this PR [1] the test operator dropped the usage of OCP Jobs for spawning of the test pods. This change updates the test-operator role so that it works with the new version of the test-operator that spawns test pods directly through the OCP Pods object. [1] openstack-k8s-operators/test-operator#266
With this PR [1] the test operator dropped the usage of OCP Jobs for spawning of the test pods. This change updates the test-operator role so that it works with the new version of the test-operator that spawns test pods directly through the OCP Pods object. [1] openstack-k8s-operators/test-operator#266
recheck |
/hold These PRs need to be merged at the same time. The idea is to first get |
/test all |
9efbbb1
to
cd18a0b
Compare
/test all |
cd18a0b
to
f0e6783
Compare
With this PR [1] the test operator dropped the usage of OCP Jobs for spawning of the test pods. This change updates the test-operator role so that it works with the new version of the test-operator that spawns test pods directly through the OCP Pods object. [1] openstack-k8s-operators/test-operator#266
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/7e844aee32a34fc49f131f3cb5ebff15 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 59m 49s |
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.
I went through the code a bit and there are many instances, where the job
words/variables are not changed into pod
. I will check the functionality next, but I wanted to call attention to this first.
Thank you @kstrenkova! Lot of renaming that needs to be done. I'll take a look. Can you also please review it from the user / functionality perspective? |
@kstrenkova: Also, please try to use the Review feature on GitHub. You can add the comments during review and then send them all and once with "Request Change". |
I actually tried to do that, but once I did Start a Review, it deleted my comments... So I added them later 😅 Maybe if I chose |
f0e6783
to
7b53cfb
Compare
7b53cfb
to
d94d50a
Compare
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.
The names should be updated.
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3dcf41acd2954779b64cc74819f4f52f ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 23m 47s |
d94d50a
to
ac42516
Compare
In this PR [1] we dropped the usage of the Jobs by the test-operator. This allows us to drop the rights for: - ServiceAccount managing & creation - Role managing & creation - RoleBinding managing & creation These rights were only needed because Jobs were required to be spawned with an extra ServiceAccount that would have elevated privileges in case the test pods need to run with privileged SecurityContext. [1] openstack-k8s-operators#266 Depends-On: openstack-k8s-operators#266
As of now, I have tested functionality of this patch as well. It seems to be working with all the CRs and the ownerRefrences are no longer |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/81fa1a540f85479889489c26e011e748 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 18m 37s |
The test-operator is using Jobs to spawn test pods even though it does not use any features of this k8s object. Plus usage of the Jobs requires creation of ServiceAccount in the target namespaces. In order to be able to create a new, SA the test-oprator has to have a rights to create new roles and rolebindings which in our case makes the attack surface larger. This patch drops the usage of Jobs and moves to Pods. Depends-On: openstack-k8s-operators/ci-framework#2604
ac42516
to
0481f26
Compare
0481f26
to
8dffcb8
Compare
It looks like the CI was not picking up the change. I've tried to merge the two commits together. Let's wait and see. |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3614a08c4caa4c9f80f7c451a63a287b ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 29m 57s |
The test-operator is using Jobs to spawn test pods even though it does
not use any features of this k8s object. Plus usage of the Jobs requires
creation of ServiceAccount in the target namespaces. In order to be able
to create a new, SA the test-oprator has to have a rights to create new
roles and rolebindings which in our case makes the attack surface
larger.
This patch drops the usage of Jobs and moves to Pods.
Depends-On: openstack-k8s-operators/ci-framework#2604