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

Implement helpers from pkg/runtime #361

Closed
wants to merge 17 commits into from
Closed

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented May 17, 2021

Ref: fluxcd/flux2#1601

(Incomplete) TODO

  • Ensure events are properly dispatched everywhere
  • Ensure debug log level messages are added
  • Add tests covering all reconcilers
  • Document and validate changes in API

@hiddeco hiddeco force-pushed the refactor-reconcilers branch 5 times, most recently from 63d65f9 to 153d8a7 Compare May 21, 2021 20:00
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>
During the writing of tests that now have end-to-end tests for both Git
implementations, an inconsistency was discovered in the `libgit2`
implementation, resulting in unaligned behavior in the parsing of tags.

This commit aligns the implementation with the `gogit` one, so that they
behave exactly the same.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
This includes the removal of the `internal/testenv` package, as this is
now available in `pkg/runtime`.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
`io/ioutil` has been deprecated since Go 1.16, see:
https://golang.org/doc/go1.16#ioutil

Signed-off-by: Hidde Beydals <hello@hidde.co>
@hiddeco
Copy link
Member Author

hiddeco commented Oct 8, 2021

Superseded by all above PRs.

@hiddeco hiddeco closed this Oct 8, 2021
@hiddeco hiddeco deleted the refactor-reconcilers branch April 19, 2022 10:36
This pull request was closed.
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