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: make controllers tolerant to spec marshalling errors #666

Merged
merged 1 commit into from
Aug 30, 2020

Conversation

jessesuen
Copy link
Member

@jessesuen jessesuen commented Aug 24, 2020

Resolves #389, #517

Uses a new, specialized "tolerant" dynamic informer which tolerates malformed objects that cannot unmarshal properly, preventing the issue where a unmarshallable object renders the entire controller unusable. When an object is discovered to be malformed, the informer will return a best effort version of the object via json unmarshalling (dropping all fields which could not be marshalled properly). This allows the controller to proceed as if the mistake was never made.

A consequence of this change is that the malformed field is silently dropped. But this is at least better behavior than stopping the controller.

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2020

Codecov Report

Merging #666 into master will decrease coverage by 0.24%.
The diff coverage is 74.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #666      +/-   ##
==========================================
- Coverage   84.95%   84.70%   -0.25%     
==========================================
  Files          89       95       +6     
  Lines        8718     8918     +200     
==========================================
+ Hits         7406     7554     +148     
- Misses        934      965      +31     
- Partials      378      399      +21     
Impacted Files Coverage Δ
utils/tolerantinformer/clusteranalysistemplate.go 72.41% <72.41%> (ø)
utils/tolerantinformer/analysisrun.go 72.97% <72.97%> (ø)
utils/tolerantinformer/analysistemplate.go 72.97% <72.97%> (ø)
utils/tolerantinformer/experiment.go 72.97% <72.97%> (ø)
utils/tolerantinformer/rollout.go 72.97% <72.97%> (ø)
utils/tolerantinformer/tollerantinformer.go 73.33% <73.33%> (ø)
utils/log/log.go 100.00% <100.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9be76fc...ab70632. Read the comment docs.

@jessesuen jessesuen force-pushed the tolerant-spec-errors branch 2 times, most recently from 62473ed to 72b84a7 Compare August 26, 2020 08:39
@jessesuen jessesuen marked this pull request as ready for review August 26, 2020 08:54
@jessesuen jessesuen force-pushed the tolerant-spec-errors branch 4 times, most recently from a02d57b to 306a4c3 Compare August 28, 2020 07:22
@jessesuen jessesuen force-pushed the tolerant-spec-errors branch from 306a4c3 to ab70632 Compare August 28, 2020 07:36
@jessesuen jessesuen requested a review from khhirani August 28, 2020 07:45
@jessesuen jessesuen assigned jessesuen and khhirani and unassigned jessesuen Aug 28, 2020
Copy link
Contributor

@khhirani khhirani left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Spec validation failed in Rollout CR
3 participants