Skip to content

Add probe for Kubernetes jobs#76

Merged
bzurkowski merged 1 commit intoopenrca:masterfrom
aleksandra-galara:Add_probe_for_jobs
May 5, 2020
Merged

Add probe for Kubernetes jobs#76
bzurkowski merged 1 commit intoopenrca:masterfrom
aleksandra-galara:Add_probe_for_jobs

Conversation

@aleksandra-galara
Copy link
Copy Markdown
Member

@aleksandra-galara aleksandra-galara commented Apr 29, 2020

It implements #26

Signed-off-by: Aleksandra Galara a.galara@samsung.com

@bzurkowski bzurkowski self-requested a review April 29, 2020 10:56
@bzurkowski bzurkowski added this to the 0.2 milestone Apr 29, 2020
@bzurkowski bzurkowski linked an issue Apr 29, 2020 that may be closed by this pull request
Copy link
Copy Markdown
Member

@bzurkowski bzurkowski left a comment

Choose a reason for hiding this comment

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

It's almost perfect. Minor remarks. Thanks!

Comment thread orca/topology/infra/k8s/probe.py Outdated

@classmethod
def get(cls, graph):
return super().get(graph, 'job', extractor.JobExtractor()) No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing EOL.

Comment thread orca/topology/infra/k8s/extractor.py Outdated
properties['name'] = reference.name
properties['uid'] = reference.uid
references.append(properties)
return references No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing EOL.

Comment thread orca/topology/infra/k8s/matcher.py Outdated

class JobToPodMatcher(Matcher):

"""Generic matcher for links between Job and Pod entities."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, it's not "generic". It's just a matcher 😉

Comment thread orca/topology/infra/k8s/__init__.py Outdated
Comment on lines +257 to +258


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Too much space.

Comment thread orca/topology/infra/k8s/extractor.py Outdated
properties['namespace'] = entity.metadata.namespace
properties['labels'] = entity.metadata.labels.copy()
properties['selector'] = entity.spec.selector.match_labels
properties['owner_references'] = self._extract_references(entity)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What we need here is a reference to a cron job - there is always only one for a given job. Maybe we could switch to a simpler field named cron_job_ref that would contain the first cron job item found in the owner_references payload?. Something similar to target_ref in HorizontalPodAutoscalerExtractor?

Copy link
Copy Markdown
Member

@bzurkowski bzurkowski left a comment

Choose a reason for hiding this comment

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

2 more remarks.

Comment thread orca/topology/infra/k8s/__init__.py Outdated
probe=probe.JobPullProbe,
linkers=[
linker.JobToPodLinker,
linker.CronJobToJobLinker
Copy link
Copy Markdown
Member

@bzurkowski bzurkowski May 5, 2020

Choose a reason for hiding this comment

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

The CronJobToJobLinker does not exist at this point - it was added in a separate PR. Therefore, we should remove the reference to the non-existent linker from probe bundles added in this PR, otherwise, it throws errors.

  1. Remove cron job linker reference.
  2. Merge this PR into master branch.
  3. Rebase cron job probe PR (Add probe for Kubernetes cron_jobs #77) with master branch to include changes related to job probe.
  4. Add missing linker references in probe bundles.

Comment thread orca/topology/infra/k8s/__init__.py Outdated
probe=probe.JobPushProbe,
linkers=[
linker.JobToPodLinker,
linker.CronJobToJobLinker
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The CronJobToJobLinker does not exist at this point. It's in a separate PR.

@aleksandra-galara aleksandra-galara force-pushed the Add_probe_for_jobs branch 2 times, most recently from d337ad2 to d050a55 Compare May 5, 2020 14:05
Signed-off-by: Aleksandra Galara <a.galara@samsung.com>
@bzurkowski bzurkowski merged commit 2be1129 into openrca:master May 5, 2020
@bzurkowski bzurkowski removed this from the 0.2 milestone Sep 9, 2020
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.

Add probe for Kubernetes jobs

2 participants