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

Print the porter version from the agent #1865

Merged
merged 1 commit into from
Jan 26, 2022

Conversation

carolynvs
Copy link
Member

What does this change

The porter agent used to print the porter version before executing the requested command. When I rewrote the bash script as a go program, that regressed and it stopped printing it out.

This adds that back.

What issue does it fix

I noticed when testing the operator that this broke. It's mostly useful for troubleshooting since the image in the porter agent pod is by digest, not a tag that you could figure out the version from.

Notes for the reviewer

N/A

Checklist

  • Did you write tests?
  • Did you write documentation?
  • Did you change porter.yaml or a storage document record? Update the corresponding schema file.
  • If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Reviewer Checklist

  • Comment with /azp run test-porter-release if a magefile or build script was modified
  • Comment with /azp run porter-integration if it's a non-trivial PR

The porter agent used to print the porter version before executing the
requested command. When I rewrote the bash script as a go program, that
regressed and it stopped printing it out.

This adds that back.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs marked this pull request as ready for review January 20, 2022 22:43
@carolynvs carolynvs requested a review from VinozzZ January 20, 2022 22:43
cmd := exec.Command(porter, "version")
cmd.Stdout = stdout
cmd.Stdout = stderr // send all non-bundle output to stderr
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why do we want to send all non-bundle output to stderr?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I've learned that it helps to decide what a user should expect to see in stdout vs stderr up-front. For example, outputting a mix of debug into and the actual command output (like a json document) to stdout makes it impossible to parse. We are still correcting that in porter in some places (as part of our instrumentation efforts).

Thinking about what a user of the operator would expect to see in stdout for the porter agent container, I am assuming that they want the output of the requested porter command. So even though the agent is doing a few extra things, like here we are printing porter version for debugging, it makes it easier for someone to separate out debug info vs the actual command output if we send porter version to stderr.

Let me know if you see a problem with sending the output of version to stderr for this container!

@carolynvs carolynvs merged commit 99c8ddd into getporter:release/v1 Jan 26, 2022
@carolynvs carolynvs deleted the agent-print-version branch January 26, 2022 15:13
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.

2 participants