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

Log the Phase Output In Its Raw Form #1432

Merged
merged 1 commit into from
May 9, 2022
Merged

Log the Phase Output In Its Raw Form #1432

merged 1 commit into from
May 9, 2022

Conversation

ihcsim
Copy link
Contributor

@ihcsim ihcsim commented May 9, 2022

Change Overview

This PR is a follow-up of a1d4240, where the format.Writer() was updated to skip the logging of all phase outputs encountered by kube.ExecOutput(). That change broke some output artifacts processing code due to the missing phase outputs. This PR adds the phase outputs back to the logs in its raw, unformatted form. The raw form ensures that later output parsing code doesn't have to handle any prefixed log metadata. This change aligns with the previous
implementation in kube.Exec() as seen at:

stdout, stderr, err := kube.Exec(cli, namespace, pod, container, cmd, nil)
format.LogWithCtx(ctx, pod, container, stdout)
format.LogWithCtx(ctx, pod, container, stderr)
if err != nil {
return nil, err
}
out, err := parseLogAndCreateOutput(stdout)

With this change, the phase outputs will be logged in their raw format in the controller stdout. E.g.,

{"ActionSet":"echo-exec-8rtsc","File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onUpdateActionSet","Line":245,"Phase":"echoExec-\u003epending","Status":"running","cluster_name":"5d29f04c-5167-4925-9f83-fb8c748f8675","hostname":"kanister-kanister-operator-74cc54d475-fspq5","level":"info","msg":"Updated ActionSet","time":"2022-05-09T20:58:22.693359774Z"}
###Phase-output###: {"key":"bar1","value":"foo1"}
###Phase-output###: {"key":"bar2","value":"foo2"}

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Test Plan

Added new unit test in pkg/format/format_test.go.

Also tested with these blueprints:

  echoTask:
    outputArtifacts:
      output:
        keyValue:
          bar: "{{ .Phases.echoTask.Output.bar }}"
    phases:
    - func: KubeTask
      name: echoTask
      args:
        namespace: "{{ .Namespace.Name }}"
        image: ghcr.io/kanisterio/kanister-tools:v9.99.9-dev
        command:
        - sh
        - "-c"
        - kando output bar "foo"
  echoExec:
    outputArtifacts:
      output:
        keyValue:
          bar1: "{{ .Phases.echoExec.Output.bar1 }}"
          bar2: "{{ .Phases.echoExec.Output.bar2 }}"
    phases:
    - func: KubeExec
      name: echoExec
      args:
        namespace: default
        pod: timer
        container: timer
        command:
        - sh
        - "-c"
        - |
          kando output bar1 "foo1"
          kando output bar2 "foo2"
  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@ihcsim ihcsim requested a review from pavannd1 May 9, 2022 21:08
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2022

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister May 9, 2022
This ensures that later output parsing code doesn't have to handle
additional log metadata prefixes. It aligns with the previous
implementation in kube.Exec() at
https://github.com/kanisterio/kanister/blob/c3ee5854d48f63073012b924d3c793ddcb58ea49/pkg/function/kube_exec.go#L94-L101

Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Kanister automation moved this from In Progress to Reviewer approved May 9, 2022
@ihcsim ihcsim added the kueue label May 9, 2022
@mergify mergify bot merged commit 96fb12e into master May 9, 2022
@mergify mergify bot deleted the fix-output-logs branch May 9, 2022 22:58
Kanister automation moved this from Reviewer approved to Done May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants