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

Merge watches for hybrid and helm #143

Merged
merged 1 commit into from
Jan 17, 2022

Conversation

varshaprasad96
Copy link
Member

@varshaprasad96 varshaprasad96 commented Dec 13, 2021

This PR does the following:

  1. Add labelSelectors to hybrid watches
  2. Remove reconcilePeriod and max-concurrent-reconciles from
    watches and instead accept it from flags.
  3. Rename ChartPath to ChartDir. ChartPath also accepts directory or a chart file.

PS: follow up would be to remove legacy/watches.yaml entirely

Signed-off-by: varshaprasad96 varshaprasad96@gmail.com

@varshaprasad96
Copy link
Member Author

cc: @fabianvf

@joelanford
Copy link
Member

joelanford commented Dec 13, 2021

Remove reconcilePeriod and max-concurrent-reconciles from watches and instead accept it from flags.

@varshaprasad96 I'm curious what the motivation is for this. IIRC, these are both optional additions to the watches.yaml file and are therefore backward compatible. Is there a reason reconcile period and max concurrent reconciles can't/shouldn't be configurable on a per-GVK basis?

pkg/watches/watches.go Outdated Show resolved Hide resolved
This PR makes the following changes:
1. Allow passing label selectors in watches
2. Remove legacy watches
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1577933045

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 88.953%

Totals Coverage Status
Change from base Build 1577508140: 0.5%
Covered Lines: 1546
Relevant Lines: 1738

💛 - Coveralls

reconciler.WithGroupVersionKind(w.GroupVersionKind),
reconciler.WithOverrideValues(w.OverrideValues),
reconciler.WithSelector(w.Selector),
reconciler.WithSelector(*w.Selector),
Copy link
Member

Choose a reason for hiding this comment

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

if w.Selector is nil does this fail with a nil pointer exception?

Copy link
Member Author

Choose a reason for hiding this comment

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

since its optional, nope. If selector is nil, we won't have predicate created to filter resources

func parsePredicateSelector(selector metav1.LabelSelector) (ctrlpredicate.Predicate, error) {

@fabianvf fabianvf merged commit 2055040 into operator-framework:main Jan 17, 2022
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.

None yet

5 participants