Skip to content

feat: render progress with talos pkg/reporter when stderr is a TTY #136

Description

@ojsef39

Problem

Interactively, topf upgrade emits one slog line per event:

time=... level=INFO msg=drain command=upgrade node=... message="evicting pod dielinke/datastorewriter-..."
time=... level=INFO msg=drain command=upgrade node=... message="evicting pod kube-system/hubble-ui-..."
time=... level=INFO msg=drain command=upgrade node=... message="evicting pod cert-manager/cert-manager-webhook-..."

talosctl upgrade shows the same information as a colorized, in-place updating status line with a spinner. For interactive use that is much easier to follow, while the slog stream is the right thing for CI.

Current state

topf already depends on github.com/siderolabs/talos/pkg/reporter, but only uses the reporter.Update type as a callback parameter and forwards each update to slog:

uncordonReport := func(u reporter.Update) {
    logger.Info("uncordon", "k8s_node", k8sNodeName, "message", u.Message)
}

reporter.New is never constructed.

talosctl builds one reporter per run and threads it through every phase:

rep := reporter.New(reporter.WithOutputMode(upgradeCmdFlags.progress.Value()))
...
imagePullInternal(ctx, c, containerdInstance, nodes, upgradeCmdFlags.upgradeImage, rep)
upgradeInternal(ctx, c, containerdInstance, nodes, upgradeCmdFlags.upgradeImage, rep)
drainNodes(ctx, c, nodes, upgradeCmdFlags.drainTimeout, rep)
uncordonNodes(ctx, c, nodeNames, upgradeCmdFlags.timeout, rep)
rebootInternal(upgradeCmdFlags.wait, upgradeCmdFlags.debug, upgradeCmdFlags.timeout, rep, opts...)

Proposal

Construct a reporter.Reporter per run and report from topf's own phases (pullInstallerImage, runUpgrade, drain/uncordon, reboot, Stabilize), keeping slog for the non-TTY case.

reporter.OutputModeAuto already does the detection (isatty.IsTerminal(os.Stderr.Fd())), so CI output stays plain with no extra work. talosctl exposes the choice as --progress AUTO|PLAIN.

Worth noting this is more than swapping a sink: topf implements the upgrade phases itself rather than calling talosctl's internals, so each phase would need to report into it.

Possibly related to #133.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions