docker-archive / deploykit

A toolkit for creating and managing declarative, self-healing infrastructure.
Apache License 2.0
2.25k stars 262 forks source link

Enrollment Options for parsing errors #826

Closed kaufers closed 6 years ago

kaufers commented 6 years ago

The enrollment controller can encounter parsing errors in either the source instances or the enrolled instances.

A source instance failing to parse only affects the removal of currently enrolled instances (since we cannot determine the unique identifier of the source instance due to the parsing error and, hence, we cannot determine if the instance is currently enrolled). When a source fails to parse there are two options:

If removal is enabled, then the source instance (that failed to parse) will be removed from the group (it is is currently enrolled).

When an enrolled instance fails to parse, this only affects the addition of source instances and there are two options:

If addition is enabled, the enrolled instance (that failed to parse) will be added again (if it is in the source instances).

This PR adds these Options to the enrollment controller configuration, the default is that removals and additions are enabled on parse errors.

chungers commented 6 years ago

I can see how this change might be controversial because this is largely a policy decision depending on what properties are used as key. Sometimes a zero value property may be the signal needed to remove something...

How about we add a field in Options to indicate the removal policy? Like MissingFieldPolicy=remove which can be set to make it more strict. Thus the enrollment controller is by default lenient?

codecov[bot] commented 6 years ago

Codecov Report

Merging #826 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #826   +/-   ##
=======================================
  Coverage   47.84%   47.84%           
=======================================
  Files          89       89           
  Lines        7945     7945           
=======================================
  Hits         3801     3801           
  Misses       3782     3782           
  Partials      362      362

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 01cf431...e9cfd2e. Read the comment docs.

kaufers commented 6 years ago

@chungers PR has been updated to parse the desired operation from the Options object, see updated description for the supported configurations.