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

feat: Create job and Watch job status for minioJob #2031

Merged
merged 13 commits into from
Mar 26, 2024

Conversation

jiuker
Copy link
Contributor

@jiuker jiuker commented Mar 13, 2024

watch the minioJob
1.handle the minioJob
2.check ref tenant
3.check sa
4.create the job
5.generate the status to minioJob according to the intervalJob status

watch the job
1.update the intervalJob status

How to test:

  1. deploy operator with envOPERATOR_STS_ENABLED = on
  2. deploy tenant
  3. deploy minioJob
apiVersion: v1
kind: ServiceAccount
metadata:
  name: mc-job-sa
---
apiVersion: sts.min.io/v1alpha1
kind: PolicyBinding
metadata:
  name: mc-job-bingding
spec:
  application:
    serviceaccount: mc-job-sa
  policies:
    - consoleAdmin
---
apiVersion: job.min.io/v1alpha1
kind: MinIOJob
metadata:
  name: minio-test-job
spec:
  serviceAccountName: mc-job-sa
  tenant:
    name: mytest-minio
  commands:
    - op: make-bucket
      args:
        name: memes
    - name: add-my-user-1
      op: admin/user/add
      args:
        user: daniel
        password: daniel123
    - name: add-my-user-2
      op: admin/user/add
      args:
        user: pedro
        password: daniel123
    - name: add-my-policy
      op: admin/policy/create
      args:
        name: memes-access
        policy: |
          {
              "Version": "2012-10-17",
              "Statement": [
                  {
                      "Effect": "Allow",
                      "Action": [
                          "s3:*"
                      ],
                      "Resource": [
                          "arn:aws:s3:::memes",
                          "arn:aws:s3:::memes/*"
                      ]
                  }
              ]
          }
    - op: admin/policy/attach
      dependsOn:
        - add-my-user-1
        - add-my-user-2
        - add-my-policy
      args:
        policy: memes-access
        user: daniel
    - op: admin/policy/attach
      dependsOn:
        - add-my-user-1
        - add-my-user-2
        - add-my-policy
      args:
        policy: memes-access
        user: pedro

Failed example:

apiVersion: job.min.io/v1alpha1
kind: MinIOJob
metadata:
 ...
status:
  commands:
    - message: ''
      name: command-0
      result: success
    - message: ''
      name: add-my-user-1
      result: success
    - message: ''
      name: add-my-user-2
      result: success
    - message: ''
      name: add-my-policy
      result: success
    - message: Job has reached the specified backoff limit
      name: command-4
      result: failed
    - message: Job has reached the specified backoff limit
      name: command-5
      result: failed
  message: Job has reached the specified backoff limit
  phase: Failed
spec:
 ...

Running example:

apiVersion: job.min.io/v1alpha1
kind: MinIOJob
metadata:
 ...
status:
  commands:
    - message: ''
      name: command-0
      result: success
    - message: ''
      name: add-my-user-1
      result: success
    - message: ''
      name: add-my-user-2
      result: success
    - message: ''
      name: add-my-policy
      result: success
    - message: ''
      name: command-4
      result: running
    - message: Job has reached the specified backoff limit
      name: command-5
      result: failed
  message: ''
  phase: Running
spec:
 ...

@cniackz
Copy link
Contributor

cniackz commented Mar 13, 2024

Yes, I like this approach. Operation Alias to MC Command function that can translate from:

    - op: make-bucket
      args:
        name: memes

to:

mc mb ALIAS/memes

@cniackz
Copy link
Contributor

cniackz commented Mar 13, 2024

Please continue with these changes; they look good to me. Just ensure that you address the lint issues and mark the pull request as ready for testing. After that, we can conduct another round of review.

Thank you very much @jiuker 🥳

@jiuker jiuker changed the title feat: valid minioJob feat: Create job and Watch job status for minioJob Mar 19, 2024
watch the minioJob
1.handle the minioJob
2.check ref tenant
3.check sa
4.create the job
5.generate the status to minioJob according to the intervalJob status

watch the job
1.update the intervalJob status
@jiuker jiuker force-pushed the feat_add_commond branch from 7b6508d to d51d09e Compare March 19, 2024 06:57
@harshavardhana harshavardhana requested review from cniackz and pjuarezd and removed request for cniackz March 20, 2024 21:06
@harshavardhana
Copy link
Member

This looks like a useful change // cc @pjuarezd

@jiuker jiuker marked this pull request as ready for review March 21, 2024 03:09
@jiuker jiuker requested review from dvaldivia and shtripat March 21, 2024 03:15
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
pkg/controller/job-controller.go Outdated Show resolved Hide resolved
@jiuker jiuker requested a review from shtripat March 22, 2024 01:58
shtripat
shtripat previously approved these changes Mar 22, 2024
Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

Verified the changes and works as expected.
Few minor ones.

pkg/utils/miniojob/minioJob.go Outdated Show resolved Hide resolved
pkg/utils/miniojob/minioJob.go Outdated Show resolved Hide resolved
pkg/utils/miniojob/minioJob.go Outdated Show resolved Hide resolved
pkg/utils/miniojob/minioJob.go Outdated Show resolved Hide resolved
pkg/utils/miniojob/minioJob.go Outdated Show resolved Hide resolved
pkg/utils/miniojob/minioJob.go Outdated Show resolved Hide resolved
pkg/utils/miniojob/minioJob.go Outdated Show resolved Hide resolved
cniackz
cniackz previously approved these changes Mar 22, 2024
apply suggestion
@jiuker jiuker dismissed stale reviews from cniackz and shtripat via f79d815 March 25, 2024 03:10
@jiuker jiuker requested review from cniackz and shtripat March 25, 2024 03:15
Copy link
Contributor

@cniackz cniackz left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants