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

fix #4081 #4082: correcting/removing versionable, also improving informOnCondition #4083

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

shawkins
Copy link
Contributor

@shawkins shawkins commented Apr 21, 2022

Description

Fix #4081
Fix #4082

For #4081 we don't want waitable under versionable - waitable requires a list, which has "no older than" not exact resourceVersion semantics.

With kubernetes 1.19+ there is support for resourceVersionMatch, which includes resourceVersionMatch=Exact, but that should probably be supported with another method - withResourceVersion(version, match) that would return something listable / informable / waitable.

The condition logic in BaseOperation was also changed to check each item, rather than assume a single to support collection contexts.

@manusa noted a related issue that is appears at least projects do not support a fieldSelector of metadata.name, which is how we implement withName when watching/informing. This will need another issue to capture this in documentation and/or add a check for multiple items when only 1 is expected.

For #4082 the BaseOperation logic will perform / test the result of a list operation, then use that as the informer initial state - that ensures that initial add events won't be processed individually.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@shawkins shawkins force-pushed the waitable branch 2 times, most recently from 7450c48 to e49929a Compare April 21, 2022 20:01
@shawkins
Copy link
Contributor Author

Considering the possibility of setting a limit, we can't simply call list - it would need to be paginated. So I took the fix a different direction - as much as possible we'll defer add events until the store is complete. The only case where this is not possible is when using limit and a non full state store - which doesn't apply to informOnCondition and related calls.

@manusa
Copy link
Member

manusa commented Apr 26, 2022

Once this is merged we can update the ProjectIT delete assertion (personal reminder)

@manusa manusa added this to the 6.0.0 milestone Apr 27, 2022
@sonarcloud
Copy link

sonarcloud bot commented Apr 28, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

70.3% 70.3% Coverage
0.0% 0.0% Duplication

@manusa manusa merged commit 03ec3b1 into fabric8io:master Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants