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

Porter should print a clear message that the bundle has changed and is being rebuilt first #500

Closed
nunix opened this issue Aug 6, 2019 · 5 comments · Fixed by #2719
Closed
Assignees
Labels
bug Oops, sorry! docs Markdown ahoy! Updates needed on porter.sh or in dev docs. help wanted Good for someone who has contributed before

Comments

@nunix
Copy link

nunix commented Aug 6, 2019

Hi team,

while testing an image with "Docker in Docker", I faced a very strange bug with the latest release:
when running porter credentials generate, Porter actually tries to install the bundle:
image

In the screenshot, you can see both attempts from Windows Powershell and WSL (ubuntu).

My "credentials" section looks like this:

credentials:
  - name: k3dip
    env: HOST_IP

Thanks a lot for your feedback :)

@vdice
Copy link
Member

vdice commented Aug 8, 2019

Hi @nunix ! I can see this behavior has been surprising. However, I believe it is a 'feature' that porter rebuilds the bundle (in some cases) prior generating credentials.

It will do so if the hash of the porter manifest (porter.yaml) doesn't match the hash included in a pre-existing bundle.json file. The thinking is that porter should perform this rebuild of the bundle.json file in these cases before generating credentials, otherwise the creds it sees to generate may be outdated (per the outdated bundle.json). cc @carolynvs for confirmation.

Perhaps we could/should think about how to reduce the UX surprise here; and/or inform the user better on what is occurring.

@nunix
Copy link
Author

nunix commented Aug 9, 2019

Hi @vdice :)
thanks a lot for your feedback and I totally understand the "rebuild" part.
My "surprise" though, it's that it started before I could enter the credentials with porter credentials generate.

If it's working as intended, and the credentials generation would come after the rebuild, I will then fix my bundle (docker part) and see if I'm asked to enter my values again (that's what I was expecting to see with porter credentials generate)

thanks again for your time and feedback

@carolynvs
Copy link
Member

Yes, that is what porter is doing. A few commands rely upon a generated file .cnab/bundle.json, those commands are: install, upgrade, uninstall, invoke and credentials generate. They rely upon the generated bundle.json because they can work with bundles made by porter or bundles created by other CNAB tools such as duffle or docker app. So porter detects when the porter.yaml has modifications and will do a rebuild before before those commands to ensure that you are not working against a stale bundle.json.

Seems like we have two good follow-ups:

  1. Add this answer here to the FAQ
  2. Add output that says that we have detected changes to the porter.yaml and are rebuilding the bundle before proceeding.

@carolynvs carolynvs added docs Markdown ahoy! Updates needed on porter.sh or in dev docs. bug Oops, sorry! help wanted Good for someone who has contributed before labels Sep 12, 2019
@carolynvs carolynvs changed the title [Bug] Porter does not generate credentials [Bug] Confusing messages about what porter is doing during generate credentials Sep 12, 2019
@squillace
Copy link

Both 1 and 2, but 2 is the critical step. No one will be suprised if porter says, hey, I have to rebuild the bundle because there have been changes.

@carolynvs carolynvs changed the title [Bug] Confusing messages about what porter is doing during generate credentials Porter should print a clear message that the bundle has changed and is being rebuilt first Mar 11, 2022
@carolynvs
Copy link
Member

carolynvs commented Sep 26, 2022

Let's print a message when we detect that the bundle is out of date and we start a rebuild. Something like "Changes have been detected to porter.yaml, rebuilding the bundle before proceeding...".

Here is the area where we determine if we need to rebuild a bundle:

if !upToDate {
fmt.Fprintln(p.Out, "Building bundle ===>")
// opts.File is non-empty, which overrides opts.CNABFile if set
// (which may be if a cached bundle is fetched e.g. when running an action)
opts.CNABFile = ""
buildOpts := BuildOptions{bundleFileOptions: opts}
buildOpts.Validate(p)
err := p.Build(ctx, buildOpts)
if err != nil {
return cnab.BundleReference{}, err
}
}

We have an integration test (requires that you compile your code with mage build before running) that validates that rebuild is working properly (filepath below). Right now we check for strings in the output like require.NotContains(t, output, "Building bundle ===>") to determine if the bundle was rebuilt. You can test your change by looking for the new message that you added when rebuilding the bundle.

tests/integration/rebuild_test.go

@carolynvs carolynvs mentioned this issue Feb 5, 2023
4 tasks
@carolynvs carolynvs self-assigned this Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Oops, sorry! docs Markdown ahoy! Updates needed on porter.sh or in dev docs. help wanted Good for someone who has contributed before
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants