Skip to content

fix: backport, divide stdout and stderr from helm to not create corrupted outputs#8333

Merged
aaron-prindle merged 2 commits into
v1from
issue-8327
Jan 19, 2023
Merged

fix: backport, divide stdout and stderr from helm to not create corrupted outputs#8333
aaron-prindle merged 2 commits into
v1from
issue-8327

Conversation

@renzodavid9

@renzodavid9 renzodavid9 commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

Fixes: #8327

Description
This PR is to backport a v2 fix: using skaffold render with a helm deployer, with helm >= 3.10.2, pulling from an OCI registry is generating a corrupted yaml output, so here we are setting two different buffers, one for stdout and another for stderr to split the messages and differentiate between the generated yaml content and the errors.

Related PRs from v2

@codecov

codecov Bot commented Jan 18, 2023

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (v1@c500fe3). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff          @@
##             v1    #8333   +/-   ##
=====================================
  Coverage      ?   68.31%           
=====================================
  Files         ?      563           
  Lines         ?    26707           
  Branches      ?        0           
=====================================
  Hits          ?    18246           
  Misses        ?     7186           
  Partials      ?     1275           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@renzodavid9 renzodavid9 marked this pull request as ready for review January 18, 2023 22:50
@renzodavid9 renzodavid9 requested a review from a team as a code owner January 18, 2023 22:50
@renzodavid9 renzodavid9 requested review from aaron-prindle and removed request for a team January 18, 2023 22:50
@aaron-prindle aaron-prindle added the kokoro:force-run forces a kokoro re-run on a PR label Jan 18, 2023
@renzodavid9

Copy link
Copy Markdown
Contributor Author

How to reproduce the issue

Environment

  • You need helm >= 3.10.2
  • You need an OCI registry. One easy way to get one running in your local machine is with Docker Registry. You only need to run:
docker run -d -p 5000:5000 --name registry registry:2

Upload a chart to your local OCI registry

helm package ./charts/
  • It will create a .tgz file. Upload that file to your local registry:
helm push ./skaffold-helm-0.1.0.tgz oci://localhost:5000/helm-charts

Reproduce the issue

  • Using skaffold v1.39.4, create a project with the following skaffold.yaml file:
apiVersion: skaffold/v2beta28
kind: Config

metadata:
  name: chart-in-oci

deploy:
  helm:
    releases:
    - name: oci-test
      remoteChart: oci://localhost:5000/helm-charts/skaffold-helm
  • Run skaffold render --output=render.yaml
  • Open the generated render.yaml file, you'll see at the beginning of the file the content that is breaking the file:
Pulled: localhost:5000/helm-charts/skaffold-helm:0.1.0
Digest: sha256:8ba9820...

Solve the issue

  • Download the PR
  • Build a new skaffold version
  • Run skaffold render --output=render.yaml
  • Open the generated render.yaml file, you'll see there is no extra content
…l` produces same output as `skaffold render &> render.yaml`

@aaron-prindle aaron-prindle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - Confirmed manually that this is working for me as well via the repro steps provided. Awesome!

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

Labels

kokoro:force-run forces a kokoro re-run on a PR size/M

2 participants