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

Job CRD #116

Merged
merged 3 commits into from
Jul 21, 2021
Merged

Job CRD #116

merged 3 commits into from
Jul 21, 2021

Conversation

stinkyfingers
Copy link
Contributor

@stinkyfingers stinkyfingers commented Jul 16, 2021

Description

Adds a Job CRD/Controller. Spec.

Quickstart:

  • make
    • make uninstall
    • make manager install - assure job CRD is live: k get crd (Missing "Jobs" CRD? kubectl apply -f config/crd/bases/theketch.io_jobs.yaml
  • ./bin/manager --enable-leader-election=false --disable-webhooks - run manager
  • kubectl apply -f sample-ketch-job.yaml (assumes you have framework "myframework" created")
  • Check jobs & logs
    • kubectl get jobs -A
    • kubectl get pods -A
    • kubectl logs <pod> -n ketch-myframework pi and kubectl logs <pod> -n ketch-myframework lister
  • kubectl delete -f sample-ketch-job.yaml (should remove the job, i.e. kubectl get jobs -A shows no "crd-test")

sample-ketch-job.yaml:

apiVersion: theketch.io/v1beta1
kind: Job
metadata:
  name: crd-test
spec:
  containers:
    - name: pi
      image: perl
      command:
        - "perl"
        - "-Mbignum=bpi"
        - "-wle"
        - "print bpi(2000)"
    - name: lister
      image: ubuntu
      command:
        - "ls"
        - "/"
  policy:
    restartPolicy: Never
  framework: myframework
  name: crd-test
  description: "i can test"
  type: Job
  version: v1
  • kubectl get jobs -n ketch-myframework
  • kubectl get pods -n ketch-myframework
  • kubectl logs <pod> -n ketch-myframework pi - for "pi" container in job
  • kubectl logs <pod> -n ketch-myframework lister - for "ls /" container in job
    Fixes # 1541

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Chore (documentation addition or typo, file relocation)

Testing

  • New tests were added with this PR that prove my fix is effective or that my feature works (describe below this bullet)
  • This change requires no testing (i.e. documentation update)

Documentation

  • All added public packages, funcs, and types have been documented with doc comments
  • I have commented my code, particularly in hard-to-understand areas

Final Checklist:

  • I followed standard GitHub flow guidelines
  • I have performed a self-review of my own code
  • My changes generate no new warnings

Additional Information (omit if empty)

Please include anything else relevant to this PR that may be useful to know.

WIP parking

adds generated job template; controller updates

addtl cleanup

refactors some chart operations; converts job.command to job.containers

adds unit tests; refactors AppCondition

fixes one merge conflict

rm unneeded tests

manifests update

stubs out Job

WIP parking

adds generated job template; controller updates

addtl cleanup

refactors some chart operations; converts job.command to job.containers

adds unit tests; refactors AppCondition

fixes one merge conflict

rm unneeded tests

reset manager controller name

add jobs to framework status
@@ -87,6 +87,10 @@ func main() {
setupLog.Error(err, "unable to set default templates")
os.Exit(1)
}
if err = storage.Update(templates.IngressConfigMapName("none"), templates.NoIngressTemplates); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

how about templates.JobConfigMapName() and templates.JobTemplates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, change made. Deciding what to call this was something I was unsure of.

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove this comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Traefik bool
Istio bool
Common bool
NoIngress bool
Copy link
Contributor

Choose a reason for hiding this comment

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

How about Job instead of NoIngress?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Renamed all NoIngress -> Job.

@@ -71,6 +71,9 @@ var (
TraefikDefaultTemplates = Templates{
Yamls: GeneratedYamls.TraefikYamls,
}
NoIngressTemplates = Templates{
Yamls: GeneratedYamls.NoIngressYamls,
Copy link
Contributor

Choose a reason for hiding this comment

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

GeneratedYamls.NoIngressYamls?

@stinkyfingers stinkyfingers merged commit 4997b2a into main Jul 21, 2021
@stinkyfingers stinkyfingers deleted the jds/job-crd branch July 21, 2021 13:33
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.

2 participants