Add `conditions` and `patch` helpers, and docs to `runtime` #101

hiddeco · 2021-05-14T20:58:16Z

This pull request introduces conditions and patch helper packages to runtime, and documents all runtime code and related API packages to a bare minimum in a hope to drive implementation efforts.

Fixes #117

This commit loosely implements the events and metrics helpers that have been newly introduced to `fluxcd/pkg/runtime`, and heavily reduces code duplication. It is in preparation of a much bigger overhaul to implement the work pending in fluxcd/pkg#101. While implementing, I ran into little annoyances that likely should be addressed before the "official" `runtime` MINOR release: - Passing `nil` every time there isn't any metadata for an event quickly becomes cumbersome; we should look into an `EventWithMetadata` and/or `EventfWithMetadata`, or some other way to _optionally_ provide metadata without annoying the consumer. - There is an inconsistency in the method names of the metric helper, i.e. `RecordReadinessMetric` vs `RecordSuspend`. We either need to append or remove the `Metric` suffix on all recording methods. Signed-off-by: Hidde Beydals <hello@hidde.co>

stefanprodan

LGTM

Thanks @hiddeco 🥇

stefanprodan · 2021-07-06T10:40:17Z

apis/meta/annotations.go

@@ -21,19 +21,19 @@ const (
 	// outside of the defined schedule. Despite the name, the value is not
 	// interpreted as a timestamp, and any change in value shall trigger a
 	// reconciliation.
-	// DEPRECATED: has been replaced by ReconcileRequestAnnotation.
+	// DEPRECATED: has been replaced by ReconcileRequestAnnotation. For


I think it's time to remove this, we've switched all controllers to ReconcileRequestAnnotation in 2020.

I would probably do this in a separate PR to not incorporate more "hidden" breaking changes in this one.

squaremo

Lots of goodies here 🎉, representing much valuable work. In particular, I want to applaud the explanations for why someone should use these things, and the examples of how to use them. The explanations are great because they describe the benefit to the user of the library, rather than just what will happen.

NB I only skimmed the stuff adapted from Kubernetes' libraries, noting that it has plenty of tests.

squaremo · 2021-07-06T12:45:55Z

runtime/conditions/getter.go

@@ -0,0 +1,302 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+Copyright 2021 The Flux authors


I think to comply with the license (https://www.apache.org/licenses/LICENSE-2.0, section 4):

"You must cause any modified files to carry prominent notices stating that You changed the files"

"You may add Your own copyright statement to Your modifications"

I'm not sure exactly what these entail, but I think it would work to use the format:

Copyright 2020 The Kubernetes Authors. This file is modified from the source at <url> to - adapt the Foo interface to better suit this project - ... Modifications copyright 2021 the Flux authors

Given keeping a list with modifications is not realistic, I opted for the following format:

This file is modified from the source at <url>, and initially adapted to <reason for copying and making modifications>.

squaremo · 2021-07-07T13:36:49Z

apis/meta/conditions.go

+// deal with more complex logic in which for example a finite error state can be observed, it is RECOMMENDED to
+// implement the StalledCondition and ReconcilingCondition.
+//
+// By doing this, observers making use of kstatus to determine the current state of the resource will have a better


<3 this explanation

apis/meta/conditions.go

runtime/controller/doc.go

runtime/predicates/doc.go

This commit adds a helper to work with the conditions of GitOps Toolkit object kinds, including but not limited to aggregration, summarization and mirroring of condition types. The work has been derived from https://github.com/kubernetes-sigs/cluster-api/tree/7478817225e0a75acb6e14fc7b438231578073d2/util/conditions but adapted to work with the `metav1.Condition` and `metav1.ConditionStatus` types. More concretely, this includes the removal of "condition severity" related functionalities, as this is not supported by the `metav1.Condition` type. The following work is still required before it can be considered ready for consumption: * Support for "negative polarity" or "normal-false" conditions; this is required to properly support the `Stalled` and `Reconciling` conditions of `meta` (and `kstatus`). * Summarization to other target conditions than `Ready`; results in a more generic API. * Support setting the `ObservedGeneration` in the status condition. Signed-off-by: Hidde Beydals <hello@hidde.co>

This commit adds support for negative polarity conditions, also known as "normal-false" or "abnormal-true" types[1], during merge and set operations. The polarity is taken into account during sort operations, and results in the following order: - P0 - Status=True, NegativePolarity=True - P1 - Status=False, NegativePolarity=False - P2 - Condition=True, NegativePolarity=False - P3 - Status=False, NegativePolarity=True - P4 - Status=Unknown This order ensures that condifitions of most importance to the user are listed first. [1]: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties Signed-off-by: Hidde Beydals <hello@hidde.co>

This commit ensures `Stalled`, `Ready` and `Reconciling` conditions are always ordered first while setting and merging conditions. This eases usage for end-users that are observing a resource via e.g. `kubectl`, and provides better integration with `kstatus`: https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md#conditions Signed-off-by: Hidde Beydals <hello@hidde.co>

Signed-off-by: Hidde Beydals <hello@hidde.co>

This includes the renaming and introduction of a set of interfaces around working with objects that follow GitOps Toolkit conventions. Signed-off-by: Hidde Beydals <hello@hidde.co>

Signed-off-by: Hidde Beydals <hello@hidde.co>

This commit makes the runtime dependency package use the API structs and interface from our own meta package, and aligns the way we make assumptions about objects being being controller-runtime compatible options with that of the other helper packages like `controller` and `conditions`. Signed-off-by: Hidde Beydals <hello@hidde.co>

Quite a lot of the errors in the package had close to the same meaning, or could be used together with a context defining wrapping error. They also assumed knowledge of component implementation details at times. This commit de-duplicates the and removes the errors in these category, and also properly documents them. Signed-off-by: Hidde Beydals <hello@hidde.co>

Signed-off-by: Hidde Beydals <hello@hidde.co>

Extension includes the default configuration of mutex profiling, and inclusion of additional endpoints. Signed-off-by: Hidde Beydals <hello@hidde.co>

Including some `go <vet|fmt>` changes. Signed-off-by: Hidde Beydals <hello@hidde.co>

Signed-off-by: Hidde Beydals <hello@hidde.co>

This commit loosely implements the events and metrics helpers that have been newly introduced to `fluxcd/pkg/runtime`, and heavily reduces code duplication. It is in preparation of a much bigger overhaul to implement the work pending in fluxcd/pkg#101. While implementing, I ran into little annoyances that likely should be addressed before the "official" `runtime` MINOR release: - Passing `nil` every time there isn't any metadata for an event quickly becomes cumbersome; we should look into an `EventWithMetadata` and/or `EventfWithMetadata`, or some other way to _optionally_ provide metadata without annoying the consumer. - There is an inconsistency in the method names of the metric helper, i.e. `RecordReadinessMetric` vs `RecordSuspend`. We either need to append or remove the `Metric` suffix on all recording methods. Signed-off-by: Hidde Beydals <hello@hidde.co>

hiddeco force-pushed the conditions-patch-helpers branch from 65412ea to 849f4a7 Compare May 14, 2021 21:29

hiddeco mentioned this pull request May 17, 2021

Set observedGeneration under condition Ready fluxcd/source-controller#362

Closed

hiddeco force-pushed the conditions-patch-helpers branch from f16559c to 959de5c Compare May 21, 2021 14:17

nab-gha mentioned this pull request Jun 10, 2021

Moving addons to higher priority layers doesn't prune from the lower layer fidelity/kraan#207

Open

hiddeco force-pushed the conditions-patch-helpers branch from 959de5c to 7ab6d82 Compare June 14, 2021 12:05

hiddeco force-pushed the conditions-patch-helpers branch 2 times, most recently from 88061f2 to a8405cb Compare June 23, 2021 11:12

stefanprodan approved these changes Jun 25, 2021

View reviewed changes

hiddeco force-pushed the conditions-patch-helpers branch 7 times, most recently from 2ac95df to 18c56eb Compare July 5, 2021 18:17

hiddeco requested review from squaremo and stefanprodan July 5, 2021 18:21

hiddeco force-pushed the conditions-patch-helpers branch 2 times, most recently from 9301197 to 57a37ad Compare July 6, 2021 10:18

hiddeco marked this pull request as ready for review July 6, 2021 10:24

stefanprodan reviewed Jul 6, 2021

View reviewed changes

squaremo approved these changes Jul 7, 2021

View reviewed changes

scottrigby mentioned this pull request Jul 7, 2021

Document runtime package and APIs, provide additional helpers #117

Closed

1 task

hiddeco mentioned this pull request Jul 7, 2021

Add TLS package and helper methods to runtime #102

Merged

scottrigby mentioned this pull request Jul 7, 2021

Roll out new unified standards to all Flux controllers fluxcd/flux2#1601

Closed

7 tasks

hiddeco added 3 commits July 7, 2021 21:46

hiddeco added 19 commits July 7, 2021 21:46

Document Kustomize API package

16fa76c

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document meta API package

ab78173

This includes the renaming and introduction of a set of interfaces around working with objects that follow GitOps Toolkit conventions. Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime client package

94067b9

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime conditions package

702679a

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime controller package

7c2b0b8

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime leaderelection package

0e9caec

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime testenv package

2d67687

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime probes package

4ee6f3b

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime transform package

95d5db3

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime events package

f248cff

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime logger package

ac747c7

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document and extend runtime pprof package

7b2d1e9

Extension includes the default configuration of mutex profiling, and inclusion of additional endpoints. Signed-off-by: Hidde Beydals <hello@hidde.co>

Align Go modules with all recent changes

65de24f

Including some `go <vet|fmt>` changes. Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime metrics package

7ec4df6

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime predicates package

06cface

Signed-off-by: Hidde Beydals <hello@hidde.co>

Document runtime patch package

25fb8b6

Signed-off-by: Hidde Beydals <hello@hidde.co>

Fixup: indents in-comment code blocks and wordings

b7eec63

Signed-off-by: Hidde Beydals <hello@hidde.co>

hiddeco force-pushed the conditions-patch-helpers branch from 57a37ad to b7eec63 Compare July 7, 2021 19:49

hiddeco changed the title ~~Add conditions and patch helper to runtime~~ Add conditions and patch helper and docs to runtime Jul 7, 2021

hiddeco merged commit 11abd07 into main Jul 7, 2021

hiddeco deleted the conditions-patch-helpers branch July 7, 2021 19:58

hiddeco changed the title ~~Add conditions and patch helper and docs to runtime~~ Add conditions and patch helpers, and docs to runtime Jul 7, 2021

hiddeco mentioned this pull request Jul 7, 2021

Remove deprecated ReconcilateAtChangedPredicate from runtime/predicates #120

Closed

hiddeco added area/runtime Controller runtime related issues and pull requests area/api API related issues and pull requests labels Jul 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `conditions` and `patch` helpers, and docs to `runtime` #101

Add `conditions` and `patch` helpers, and docs to `runtime` #101

hiddeco commented May 14, 2021 •

edited

Loading

stefanprodan left a comment

stefanprodan Jul 6, 2021

hiddeco Jul 6, 2021

squaremo left a comment

squaremo Jul 6, 2021

hiddeco Jul 7, 2021 •

edited

Loading

squaremo Jul 7, 2021

Add conditions and patch helpers, and docs to runtime #101

Add conditions and patch helpers, and docs to runtime #101

Conversation

hiddeco commented May 14, 2021 • edited Loading

stefanprodan left a comment

Choose a reason for hiding this comment

stefanprodan Jul 6, 2021

Choose a reason for hiding this comment

hiddeco Jul 6, 2021

Choose a reason for hiding this comment

squaremo left a comment

Choose a reason for hiding this comment

squaremo Jul 6, 2021

Choose a reason for hiding this comment

hiddeco Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

squaremo Jul 7, 2021

Choose a reason for hiding this comment

Add `conditions` and `patch` helpers, and docs to `runtime` #101

Add `conditions` and `patch` helpers, and docs to `runtime` #101

hiddeco commented May 14, 2021 •

edited

Loading

hiddeco Jul 7, 2021 •

edited

Loading