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

Add Kubernetes Job support to Weave Scope #3609

Merged
merged 4 commits into from
May 20, 2019
Merged

Add Kubernetes Job support to Weave Scope #3609

merged 4 commits into from
May 20, 2019

Conversation

satyamz
Copy link
Contributor

@satyamz satyamz commented May 18, 2019 •
edited
Loading

This covers:

  • Implementation of the Job kubernetes resource in probe
  • Reporter changes for the Job
  • Add Describe Control to the Job
  • Pass Job object to the kube controller renderer
1. Jobs in the controllers topology (In the square shape)

Screenshot from 2019-05-19 02-36-29

2. Job weave scope native description

Screenshot from 2019-05-19 02-44-47

3. Job control option, kubectl describe

Screenshot from 2019-05-19 02-45-20

TODO:

  • Need to decide the shape for the Job.

fixes: #3362
Signed-off-by: Satyam Zode satyamzode@gmail.com

@satyamz
Add Kubernetes Job support to Weave Scope
36361d6 
This covers:
- Implementation of the job resource in probe
- Reporter changes for the job
- Add Describe Control to the job
- Pass job object to the kube controller renderer

ToDo:
- Need to decide the shape for the Job.

Signed-off-by: Satyam Zode <satyamzode@gmail.com>
@satyamz satyamz mentioned this pull request May 18, 2019
Closed
2 tasks
@satyamz
Copy link
Contributor Author

satyamz commented May 18, 2019

Hi @fbarl, @bia I could use your help on deciding shape for the Job. Any ideas? 😄

@satyamz satyamz requested review from qiell, bboreham and fbarl May 18, 2019 21:25
@qiell
Copy link
Contributor

qiell commented May 20, 2019

Just an idea, we can have the job shape similar to cronjob but with dotted lines.

@bia
Copy link
Contributor

bia commented May 20, 2019

That's a good idea, @qiell. 😄
@fbarl and I talked through the options and we think that we should do exactly what you said : dotted triangles for jobs.

@satyamz
Copy link
Contributor Author

satyamz commented May 20, 2019

Thanks @qiell, @bia, @fbarl !!

@fbarl We are raising PR in ui-components for the above change. After that, can you please bump up the version here?

@fbarl
Copy link
Contributor

fbarl commented May 20, 2019

We are raising PR in ui-components for the above change.

That's awesome, thanks!

After that, can you please bump up the version here?

Sure, you can also add me as a reviewer for your PR in ui-components.

fbarl
fbarl reviewed May 20, 2019
View reviewed changes
Copy link
Contributor

@fbarl fbarl left a comment

Choose a reason for hiding this comment

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

The code LGTM! 👍

I'll test out the behavior once the final changes are in.

report/report.go Outdated
@@ -297,6 +302,10 @@ func MakeReport() Report {
WithTag(Camera).
WithLabel("volume snapshot data", "volume snapshot data"),

Job: MakeTopology().
WithShape(StorageSheet).
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reminder to replace this with DottedTriangle once we get the new shape from ui-components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes!

@fbarl
Copy link
Contributor

fbarl commented May 20, 2019

@qiell The new DottedTriangle shape should now be available if you bump ui-components to v0.20.0 (https://github.com/weaveworks/ui-components/commits/master) - from what I can tell, none of the other changes in that repo will have an effect on Scope so it's safe to do it directly in this PR 👍

satyamz added 2 commits May 20, 2019 16:55
@satyamz
Upgrade the weaveworks ui-components to 0.20.0
6b75856 
- Include shape for dottedtriangle

Signed-off-by: Satyam Zode <satyamzode@gmail.com>
@satyamz
Change shape of K8s Job to dottedtriangle
6c4b01d 
Signed-off-by: Satyam Zode <satyamzode@gmail.com>
@satyamz
Copy link
Contributor Author

satyamz commented May 20, 2019

Hi @fbarl , @bia

This is how it looks now. Thanks!!

image

qiell
qiell approved these changes May 20, 2019
View reviewed changes
Copy link
Contributor

@qiell qiell left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM 👍

fbarl
fbarl approved these changes May 20, 2019
View reviewed changes
Copy link
Contributor

@fbarl fbarl left a comment

Choose a reason for hiding this comment

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

It worked nicely for me, good job! 👍

Just one small thing before merging - could you please add the get verb to the batch API group for jobs in examples/k8s/cluster-role.yaml? Otherwise the describe action is forbidden when trying to run the examples.

@satyamz
Add get verb to the batch apigroup in cluster role
3e41793 
Signed-off-by: Satyam Zode <satyamzode@gmail.com>
@satyamz
Copy link
Contributor Author

satyamz commented May 20, 2019

Just one small thing before merging - could you please add the get verb to the batch API group for jobs in examples/k8s/cluster-role.yaml? Otherwise the describe action is forbidden when trying to run the examples.

Good catch @fbarl! I updated the examples.

@satyamz satyamz merged commit 392862a into master May 20, 2019
@satyamz satyamz deleted the k8s-job-support branch May 20, 2019 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiell qiell qiell approved these changes

@fbarl fbarl fbarl approved these changes

@bboreham bboreham Awaiting requested review from bboreham bboreham is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Kubernetes Job workload
4 participants
@satyamz @qiell @bia @fbarl