Skip to content

wip: attempt to simplify workflows handling #2555

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
Closed
wants to merge 91 commits into from
Closed

wip: attempt to simplify workflows handling #2555

wants to merge 91 commits into from

Conversation

metacosm
Copy link
Collaborator

  • refactor: remove now unneeded NameSetter interface
  • refactor: simplify workflow handling
  • refactor: initial work to combine ManagedWorkflow and regular one

csviri and others added 30 commits October 10, 2024 10:42
@csviri @metacosm
chore: set version to 5.0.0-SNAPSHOT (#2200)
ee91509 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: java version minimal 11, tested on 21 (#2207)
623bd0c 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: remove deprecated EventFilter (#2208)
034295a 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
docs: 5.0 migration guide skeleton (#2210)
a27f561 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: managed dependent reconciliation results not optional (#2212)
937edfb 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: remove deprecated RetryConfiguration (#2211)
dc92d22 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: JDK client is now the default (#2235)
c307fa4 
Signed-off-by: Attila Mészáros <[email protected]>
@metacosm
fix: update SCM information (#2237)
c037f2a 
Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: move name is directly to dependent resource (#2250)
3ada20e 
* feat: move name is directly to dependent resource

- use this name when throwing aggregate exception

Signed-off-by: Attila Mészáros <[email protected]>

* refactor to use a dedicated interface for setting the name

Signed-off-by: Attila Mészáros <[email protected]>

* refactor: add default implementation for name() (#2255)

Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>

---------

Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Chris Laprun <[email protected]>
Co-authored-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
format
e847a30 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
fix: test after rebase on master (#2270)
54a1e76 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: use java 17 as baseline (#2271)
ac73110 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: matcher always considers metadata (#2273)
595199f 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: API to check if next reconciliation is imminent (#2272)
91c554f 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: Workflow extracted to a separate annotation (#2274)
9e725c3 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
fix: pom properties cleanup (#2280)
713f9ea 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: remove EventSourceInitializer (#2257)
93b1679 
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Chris Laprun <[email protected]>
Co-authored-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@metacosm
refactor: make loadFromProperties private (#2287)
8d6a648 
Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: distinguish resources based on desired state (#2252)
bbf0fbe 
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: explicit workflow invocation (#2289)
e5f15dc 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: use ssa for status update by default from UpdateControl (#2278)
07ec082 
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@metacosm
refactor: make WorkflowSpec an interface (#2300)
04678c6 
Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: replace current formatting plugins with spotless plugin (#2302)
639b439 
Signed-off-by: Attila Mészáros <[email protected]>
@metacosm
refactor: make ManagedWorkflowSupport public to be able to reuse it (#…
c161b8b 
…2308)

Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
fix: package fix leader election sample (#2303)
2f3c453 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: remove resource discriminator (#2299)
45d68bf 
Signed-off-by: Attila Mészáros <[email protected]>
@metacosm
refactor: remove deprecated getTerminationTimeoutSeconds and associat…
cad5d1d 
…ed (#2297)

Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: additional logging to mysql schema e2e test (#2320)
1ea47c5 
Signed-off-by: Attila Mészáros <[email protected]>
@metacosm
feat: handle clustered resource on secondary to primary mapper init (#…
879e070 
…2321)

Fixes #2311

Overriding getPrimaryResourceType should allow to make things work even
in deeper hierarchies.

Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: using SSA for finalizer and primary patch (#2305)
21e12ad 
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
csviri and others added 11 commits October 10, 2024 11:25
@csviri @metacosm
fix: cleanup after rebase on main
755091c 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: context getSecondary resource is activation condition aware (#2532
1b7b2d5 
)


Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
feat: e2e test to demonstrate namespace deletion problem fix (#2528)
1189f24 

Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
docs: remove javadoc todos (#2541)
417de6d 
Signed-off-by: Attila Mészáros <[email protected]>
@10000-ki @metacosm
feat: print diff resource as yaml (#2542)
d3390c9
@csviri @metacosm
fix: integration test timeout issue (#2547)
48c36c1 
Signed-off-by: Attila Mészáros <[email protected]>
@csviri @metacosm
improve: naming target resource selector method (#2546)
0be0f42
@bachmanity1 @metacosm
feat: improve diff logging (#2544)
68832a0 
* imporve diff logging

Signed-off-by: bachmanity1 <[email protected]>

* compute diff only when actual doesn't match desired

Signed-off-by: bachmanity1 <[email protected]>

* slight improvements

Signed-off-by: bachmanity1 <[email protected]>

* increase context size

Signed-off-by: bachmanity1 <[email protected]>

* fix style

Signed-off-by: bachmanity1 <[email protected]>

* calculate diff only if debug is enabled

Signed-off-by: bachmanity1 <[email protected]>

* print actual resources when trace is enabled

Signed-off-by: bachmanity1 <[email protected]>

* use java-diff-utils

Signed-off-by: bachmanity1 <[email protected]>

* add unit tests

---------

Signed-off-by: bachmanity1 <[email protected]>
@metacosm
fix: race condition in workflow reconciler (#2549)
3dd77c1 
Signed-off-by: Chris Laprun <[email protected]>
@metacosm
feat(ci): split integration tests execution by category (#2548)
22a2b21 
* feat(build): split ITs by category, parallel CRD generation

Also avoid generating CRDs when not needed by default, summarizes test
results.

* chore(deps): bump actions-setup-minikube to 2.13.0
* feat: default to use vertx client
* chore(ci): reduce tested combinations, only run client tests on baseapi
* fix: increase deletion timeout

---------

Signed-off-by: Chris Laprun <[email protected]>
@metacosm
refactor: rename method more appropriately (#2554)
e8d44c4 
Signed-off-by: Chris Laprun <[email protected]>
@metacosm metacosm self-assigned this Oct 18, 2024
@metacosm metacosm requested review from csviri and xstefank October 18, 2024 17:32
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2024
xstefank
xstefank reviewed Oct 22, 2024
View reviewed changes
Copy link
Collaborator

@xstefank xstefank left a comment

Choose a reason for hiding this comment

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

Other than todos I don't see anything wrong.

...n/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java
}

this.workflow = new DefaultWorkflow<>(alreadyResolved.values(), false, hasCleaner);
this.orderedSpecs = orderedSpecs; // todo: remove
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand these todos in this class. Do you still plan to work on them in this PR?

@csviri
Copy link
Collaborator

csviri commented Oct 25, 2024

@metacosm I would prefer to review this together, I fail to see how this simplifies things.

@csviri
Copy link
Collaborator

csviri commented Oct 25, 2024

Might be also better to do such refactorings step by step with limited scoped PR- if possible.

@metacosm
Copy link
Collaborator Author

This isn't done yet so, indeed, this doesn't really simplify things at the moment… also, I'm not sure the individual steps will make any difference since they will only make sense once the whole is completed (if it can actually be achieved as expected, which I'm not yet sure of as this is still rather exploratory).

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 28, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 30, 2024
@metacosm
Copy link
Collaborator Author

metacosm commented Nov 5, 2024

Replaced by #2566

@metacosm metacosm closed this Nov 5, 2024
@metacosm metacosm deleted the clean-workflows branch November 12, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xstefank xstefank xstefank left review comments

@csviri csviri Awaiting requested review from csviri

Assignees

@metacosm metacosm

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@metacosm @csviri @xstefank @openshift-merge-robot @10000-ki @matteriben @bachmanity1