fix(helmwatch): default CoreFactory in production so logtailer actually runs (#305 follow-up)

In production, handler.New() never assigns h.coreFactory, so phase1_watch
left cfg.CoreFactory == nil. helmwatch.NewWatcher had no default for
CoreFactory (DynamicFactory had one) → the helm-controller log tailer was
never launched → every PhaseComponentLog event was silently dropped.

Result on the live otech cluster: the bridge fix in #307 worked
correctly for state transitions, but the GitLab-style log viewer only
ever saw the synthetic [seeded] / [<state>] anchor lines because the
upstream emission path of raw helm-controller stdout was disconnected.

Fix:
  - helmwatch.NewWatcher defaults CoreFactory to
    NewKubernetesClientFromKubeconfig (mirroring the existing
    DynamicFactory default).
  - New regression test TestNewWatcher_DefaultsBothFactories asserts
    both factories are non-nil after construction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
hatice yildiz 2026-04-30 20:15:21 +02:00
parent add7f47ba1
commit c526187a39
2 changed files with 36 additions and 0 deletions

View File

@ -379,6 +379,15 @@ func NewWatcher(cfg Config, emit Emit) (*Watcher, error) {
if cfg.DynamicFactory == nil {
cfg.DynamicFactory = NewDynamicClientFromKubeconfig
}
if cfg.CoreFactory == nil {
// Production default: same kubeconfig → typed clientset for the
// helm-controller log tailer. Without this fallback, every
// `bp-*` HelmRelease's raw helm-controller stdout would be
// dropped — the FE log viewer would only ever render synthetic
// `[seeded]` / `[<state>]` summary lines. See issue #305.
// Tests still inject a fake.NewSimpleClientset via Config.CoreFactory.
cfg.CoreFactory = NewKubernetesClientFromKubeconfig
}
cfg.applyDefaults()
return &Watcher{
cfg: cfg,

View File

@ -608,6 +608,33 @@ func TestWatch_NilEmitRejected(t *testing.T) {
}
}
// TestNewWatcher_DefaultsBothFactories locks in the production wiring:
// when the caller leaves DynamicFactory AND CoreFactory unset, NewWatcher
// fills them with the real-cluster constructors so the dynamic informer
// runs AND the helm-controller log tailer runs (issue #305 — without the
// CoreFactory default, every PhaseComponentLog event would be silently
// dropped because the tailer never started in production).
func TestNewWatcher_DefaultsBothFactories(t *testing.T) {
rec := &recorder{}
// Provide a parseable kubeconfig YAML so NewWatcher doesn't reject
// it on the empty-string check; the factories are NOT invoked here
// (Watch() invokes them) so the kubeconfig contents don't have to
// resolve to a real cluster.
cfg := Config{
KubeconfigYAML: "apiVersion: v1\nkind: Config\nclusters: []\ncontexts: []\nusers: []\n",
}
w, err := NewWatcher(cfg, rec.emit)
if err != nil {
t.Fatalf("NewWatcher: %v", err)
}
if w.cfg.DynamicFactory == nil {
t.Errorf("DynamicFactory must default to NewDynamicClientFromKubeconfig in production")
}
if w.cfg.CoreFactory == nil {
t.Errorf("CoreFactory must default to NewKubernetesClientFromKubeconfig in production (regression for #305 — log tailer was disabled in prod)")
}
}
// TestWatch_OnlyEmitsOnTransition proves the de-dup branch: a second
// informer Update event for the same HelmRelease with no state change
// (e.g. a status subresource patch from helm-controller's