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

Refresh flattened references when interpolating values #8

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

simu
Copy link

@simu simu commented Jul 25, 2022

Sometimes using nested references to lookup values for parameters based on a parameter which is set differently in the hierarchy for different nodes leads to a ResolveError.

The original issue was discovered in Project Syn component backup-k8up, which supports being compiled multiple times with
different configurations in multiple Kapitan targets. If the two configurations use different values for the parameter backup_k8up.majorVersion, the nested references which use this parameter to look up values for the appropriate major version sometimes use stale flattened refs when checking whether all nested references are resolved causing a ResolveError.

From what I was able to determine this happens because nested references are flattened in RefItem.assembleRefs() which can cause interpolation to operate with stale flattened references if the same RefItem is part of the parameters structure for multiple nodes.

We address the problem by calling value.assembleRefs(self._base) for each value which is processed in Parameters._interpolate_inner(). This fixes the issue both in the included minimal test case and our real-world case when rendering the reclass inventory for Project Syn clusters which include component backup-k8up as described in the component v2 to v3 migration how-to.

I haven't been able to determine exactly how a RefItem object can be part of multiple nodes and therefore I'm not sure if it's expected that the same RefItem object can be part of multiple nodes or not. Unfortunately this means that I can't tell if my fix is just masking another bug somewhere which wrongly adds the same RefItem to the parameters of multiple nodes.

Sometimes using nested references to lookup values for parameters based
on a parameter which is set differently in the hierarchy for different
nodes leads to a ResolveError.

The original issue was discovered in Project Syn component
[backup-k8up], which supports being compiled multiple times with
different configurations in multiple Kapitan targets. If the two
configurations use different values for the parameter
`backup_k8up.majorVersion`, the nested references which use this
parameter to look up values for the appropriate major version sometimes
use stale flattened refs when checking whether all nested references are
resolved causing a `ResolveError`.

From what I was able to determine this happens because nested references
are flattened in `RefItem.assembleRefs()` which can cause interpolation
to operate with stale flattened references if the same `RefItem` is part
of the parameters structure for multiple nodes.

We address the problem by calling `value.assembleRefs(self._base)` for
each value which is processed in `Parameters._interpolate_inner()`. This
fixes the issue both in the included minimal test case and our
real-world case when rendering the reclass inventory for Project Syn
clusters which include component [backup-k8up] as described in the
[component v2 to v3 migration how-to].

I haven't been able to determine exactly how a `RefItem` object can be
part of multiple nodes and therefore I'm not sure if it's expected that
the same `RefItem` object can be part of multiple nodes or not.
Unfortunately this means that I can't tell if my fix is just masking
another bug somewhere which wrongly adds the same `RefItem` to the
parameters of multiple nodes.

[backup-k8up]: https://github.com/projectsyn/component-backup-k8up.git
[component migration how-to]: https://hub.syn.tools/backup-k8up/how-tos/upgrade-v2-v3.html#_steps_to_run_two_instances_at_once
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.

1 participant