fix: add debug logging, unit tests, docs, and fix integration test for subhelmfile selector skip
- Add debug log when skipping subhelmfile due to selector conflict - Add TestSubhelmfileSelectorsConflict with 11 cases for direct unit coverage - Document the selector-based subhelmfile skip optimization in docs/index.md - Fix integration test: use 'app' label key instead of reserved 'name' key (GetReleasesWithLabels overwrites labels["name"] with the release name) Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
parent
fc475022ca
commit
5ca2cbb68d
|
|
@ -1523,6 +1523,7 @@ helmfiles:
|
|||
```
|
||||
|
||||
* When a selector is specified, only this selector applies and the parents or CLI selectors are ignored.
|
||||
* When CLI selectors are provided (e.g. `helmfile -l name=b sync`) and a subhelmfile has explicit selectors that are provably incompatible (same key, different value), that subhelmfile is **skipped entirely** without loading or rendering it. For example, with `-l name=b`, a subhelmfile with `selectors: [name=a]` will be skipped since no release could match both. This optimization does not apply when `selectorsInherited: true` is set or when no CLI selectors are provided. Use `--debug` to see log messages about skipped subhelmfiles.
|
||||
* When not selector is specified there are 2 modes for the selector inheritance because we would like to change the current inheritance behavior (see [issue #344](https://github.com/roboll/helmfile/issues/344) ).
|
||||
* Legacy mode, sub-helmfiles without selectors inherit selectors from their parent helmfile. The initial helmfiles inherit from the command line selectors.
|
||||
* explicit mode, sub-helmfile without selectors do not inherit from their parent or the CLI selector. If you want them to inherit from their parent selector then use `selectorsInherited: true`. To enable this explicit mode you need to set the following environment variable `HELMFILE_EXPERIMENTAL=explicit-selector-inheritance` (see [experimental](#experimental-features)).
|
||||
|
|
|
|||
|
|
@ -1008,6 +1008,7 @@ func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, def
|
|||
anyMatched := false
|
||||
for i, m := range st.Helmfiles {
|
||||
if subhelmfileSelectorsConflict(a.Selectors, m) {
|
||||
a.Logger.Debugf("skipping subhelmfile %q: CLI selectors %v conflict with subhelmfile selectors %v", m.Path, a.Selectors, m.Selectors)
|
||||
continue
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -851,6 +851,89 @@ releases:
|
|||
runFilterSubHelmFilesTests(desiredTestcases, files, t, "2nd inherits")
|
||||
}
|
||||
|
||||
func TestSubhelmfileSelectorsConflict(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
cliSelectors []string
|
||||
spec state.SubHelmfileSpec
|
||||
conflict bool
|
||||
}{
|
||||
{
|
||||
name: "no CLI selectors",
|
||||
cliSelectors: nil,
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"name=a"}},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "no subhelmfile selectors",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: nil},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "selectorsInherited true",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"name=a"}, SelectorsInherited: true},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "conflicting selectors",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"name=a"}},
|
||||
conflict: true,
|
||||
},
|
||||
{
|
||||
name: "matching selectors",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "b.yaml", Selectors: []string{"name=b"}},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "different keys no conflict",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"env=prod"}},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "empty CLI selectors slice",
|
||||
cliSelectors: []string{},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"name=a"}},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "empty subhelmfile selectors slice",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{}},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "all pairs conflict with multiple subhelmfile selectors",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"name=a", "name=c"}},
|
||||
conflict: true,
|
||||
},
|
||||
{
|
||||
name: "one matching pair among multiple subhelmfile selectors",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"name=a", "name=b"}},
|
||||
conflict: false,
|
||||
},
|
||||
{
|
||||
name: "selectorsInherited false with conflicting selectors still conflicts",
|
||||
cliSelectors: []string{"name=b"},
|
||||
spec: state.SubHelmfileSpec{Path: "a.yaml", Selectors: []string{"name=a"}, SelectorsInherited: false},
|
||||
conflict: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := subhelmfileSelectorsConflict(tt.cliSelectors, tt.spec)
|
||||
assert.Equal(t, tt.conflict, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func runFilterSubHelmFilesTests(testcases []struct {
|
||||
label string
|
||||
expectedReleases []string
|
||||
|
|
|
|||
|
|
@ -13,27 +13,27 @@ trap cleanup_issue_2544 EXIT
|
|||
|
||||
test_start "issue-2544: subhelmfiles skipped when selectors conflict with CLI"
|
||||
|
||||
# --- Test 1: -l name=b should only produce output from subhelmfile-b ---------------------------
|
||||
# --- Test 1: -l app=b should only produce output from subhelmfile-b ---------------------------
|
||||
|
||||
info "Running helmfile template -l name=b"
|
||||
${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l name=b \
|
||||
info "Running helmfile template -l app=b"
|
||||
${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l app=b \
|
||||
> "${issue_2544_tmp_dir}/output-b.txt" 2>&1 || {
|
||||
cat "${issue_2544_tmp_dir}/output-b.txt"
|
||||
fail "helmfile template -l name=b failed"
|
||||
fail "helmfile template -l app=b failed"
|
||||
}
|
||||
|
||||
# Should contain release-b
|
||||
grep -q "release-b" "${issue_2544_tmp_dir}/output-b.txt" \
|
||||
|| fail "output should contain release-b (matching selector name=b)"
|
||||
|| fail "output should contain release-b (matching selector app=b)"
|
||||
|
||||
# Should NOT contain release-a or release-c (their subhelmfiles have incompatible selectors)
|
||||
grep -q "release-a" "${issue_2544_tmp_dir}/output-b.txt" \
|
||||
&& fail "output should NOT contain release-a (subhelmfile selectors name=a conflict with name=b)"
|
||||
&& fail "output should NOT contain release-a (subhelmfile selectors app=a conflict with app=b)"
|
||||
|
||||
grep -q "release-c" "${issue_2544_tmp_dir}/output-b.txt" \
|
||||
&& fail "output should NOT contain release-c (subhelmfile selectors name=c conflict with name=b)"
|
||||
&& fail "output should NOT contain release-c (subhelmfile selectors app=c conflict with app=b)"
|
||||
|
||||
info "PASS: -l name=b produces only release-b"
|
||||
info "PASS: -l app=b produces only release-b"
|
||||
|
||||
# --- Test 2: no selector should produce all releases -----------------------------------------
|
||||
|
||||
|
|
@ -53,25 +53,25 @@ grep -q "release-c" "${issue_2544_tmp_dir}/output-all.txt" \
|
|||
|
||||
info "PASS: no selector produces all releases"
|
||||
|
||||
# --- Test 3: -l name=a should skip subhelmfiles with selectors name=b and name=c ---------------
|
||||
# --- Test 3: -l app=a should skip subhelmfiles with selectors app=b and app=c ---------------
|
||||
|
||||
info "Running helmfile template -l name=a"
|
||||
${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l name=a \
|
||||
info "Running helmfile template -l app=a"
|
||||
${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l app=a \
|
||||
> "${issue_2544_tmp_dir}/output-a.txt" 2>&1 || {
|
||||
cat "${issue_2544_tmp_dir}/output-a.txt"
|
||||
fail "helmfile template -l name=a failed"
|
||||
fail "helmfile template -l app=a failed"
|
||||
}
|
||||
|
||||
grep -q "release-a" "${issue_2544_tmp_dir}/output-a.txt" \
|
||||
|| fail "output should contain release-a (matching selector name=a)"
|
||||
|| fail "output should contain release-a (matching selector app=a)"
|
||||
|
||||
grep -q "release-b" "${issue_2544_tmp_dir}/output-a.txt" \
|
||||
&& fail "output should NOT contain release-b (subhelmfile selectors name=b conflict with name=a)"
|
||||
&& fail "output should NOT contain release-b (subhelmfile selectors app=b conflict with app=a)"
|
||||
|
||||
grep -q "release-c" "${issue_2544_tmp_dir}/output-a.txt" \
|
||||
&& fail "output should NOT contain release-c (subhelmfile selectors name=c conflict with name=a)"
|
||||
&& fail "output should NOT contain release-c (subhelmfile selectors app=c conflict with app=a)"
|
||||
|
||||
info "PASS: -l name=a produces only release-a"
|
||||
info "PASS: -l app=a produces only release-a"
|
||||
|
||||
trap - EXIT
|
||||
test_pass "issue-2544: subhelmfiles skipped when selectors conflict with CLI"
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
helmfiles:
|
||||
- path: subhelmfile-a.yaml
|
||||
selectors:
|
||||
- name=a
|
||||
- app=a
|
||||
- path: subhelmfile-b.yaml
|
||||
selectors:
|
||||
- name=b
|
||||
- app=b
|
||||
- path: subhelmfile-c.yaml
|
||||
selectors:
|
||||
- name=c
|
||||
- app=c
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ releases:
|
|||
- name: release-a
|
||||
chart: ../../../charts/raw
|
||||
labels:
|
||||
name: a
|
||||
app: a
|
||||
values:
|
||||
- templates:
|
||||
- |
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ releases:
|
|||
- name: release-b
|
||||
chart: ../../../charts/raw
|
||||
labels:
|
||||
name: b
|
||||
app: b
|
||||
values:
|
||||
- templates:
|
||||
- |
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ releases:
|
|||
- name: release-c
|
||||
chart: ../../../charts/raw
|
||||
labels:
|
||||
name: c
|
||||
app: c
|
||||
values:
|
||||
- templates:
|
||||
- |
|
||||
|
|
|
|||
Loading…
Reference in New Issue