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

Regression in v0.30.0-pre3 ? #1850

Closed
mhdawson opened this issue Jul 26, 2023 · 18 comments
Closed

Regression in v0.30.0-pre3 ? #1850

mhdawson opened this issue Jul 26, 2023 · 18 comments
Labels
status/resolved Support issues that have been resolved. type/bug Issue that reports an unexpected behaviour.
Milestone

Comments

@mhdawson
Copy link

mhdawson commented Jul 26, 2023

Summary

Running the smoke test for the ubi-builder-base which uses the ubi-ndoejs-extension that we have been working on works with v0.30.0-pre2 but not with v0.30.0-pre3.

The error seems to be:

           [restorer] ERROR: failed to read builder image: failed to create cache directory: mkdir /kaniko/cache: permission denied
            ERROR: failed to build: executing lifecycle: failed with status code: 1

Reproduction

Steps
  1. checkout this GitHub repo- https://github.com/paketo-community/builder-ubi-base
  2. cd builder-ubi-base
  3. edit scripts/.util/tools.json to change v0.30.0-pre3 to v0.30.0-pre2
  4. run "bash scripts/smoke.sh"
Current behavior

Fails with the error shown above.

Expected behavior

container builds and smoke tests passes


Environment

pack info
<!--- Run `pack report` and copy output here. -->
user1@fedora .bin]$ ./pack report
Pack:
  Version:  0.30.0-pre3+git-cc6b001.build-4846
  OS/Arch:  linux/amd64

Default Lifecycle Version:  0.17.0-rc.3

Supported Platform APIs:  0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12

Config:
  default-builder-image = "[REDACTED]"
  experimental = true
  layout-repo-dir = "/home/user1/.pack/layout-repo"
docker info
@mhdawson mhdawson added status/triage Issue or PR that requires contributor attention. type/bug Issue that reports an unexpected behaviour. labels Jul 26, 2023
@pacostas
Copy link

I get the error you mentioned on both v0.30.0-pre3 and v0.30.0-pre2

I used this branch initial-version from this repo https://github.com/mhdawson/builder-ubi-base

pack report

$ pack report
Pack:
  Version:  0.0.0+git-cc6b001
  OS/Arch:  linux/amd64

Default Lifecycle Version:  0.17.0-rc.3

Supported Platform APIs:  0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12

Config:
  default-builder-image = "[REDACTED]"
  experimental = true
  layout-repo-dir = "/home/costas/.pack/layout-repo"

@natalieparellano
Copy link
Member

@mhdawson @pacostas thanks for this - I only see a README.md in that repo. Are you perhaps working off a different version?

I do see that the default lifecycle version from pack is 0.17.0-rc.3 - can you also confirm that is the version used in the build? Earlier versions of the lifecycle did not ensure that the kaniko directory was writeable by the CNB user, but this was (we thought) fixed in https://github.com/buildpacks/lifecycle/pull/1117/files#diff-73763e61f2242eb741045f9665ef775d854ec4f0ed00d18d6ad6518d0833f6b3R72

@pacostas
Copy link

pacostas commented Jul 28, 2023

About the repository: is this one git remote add mhdawson https://github.com/mhdawson/builder-ubi-base and checkout on this branch git checkout initial-version

@natalieparellano
Copy link
Member

natalieparellano commented Jul 28, 2023

@pacostas thanks for the tip! From this branch, I see the lifecycle version in use is 0.17.0-pre.2, which is quite old. I've updated locally to 0.17.0-rc.3 and I don't see /kaniko/cache: permission denied anymore but I do see a new error:

        [restorer] Pulling builder image metadata for index.docker.io/library/testbuilder:latest...
        [restorer] ERROR: failed to pull builder image: failed to get remote image

Does the smoke script ensure that the builder image is saved to a registry? This is needed in order to generate the manifest required for extension with kaniko (previous versions of pack and the lifecycle had bugs that resulted in silent failure to pull the builder image manifest). When I'm testing locally I always push the builder to a registry:2 instance on localhost:5000.

I should note, if your extensions perform run image switching (only, without extension) there is a bug in lifecycle 0.17.0-rc.3 that was fixed in buildpacks/lifecycle#1159. We are hoping to cut a lifecycle 0.17.0-rc.4 next week, once the specs have been finalized.

@mhdawson
Copy link
Author

Thanks for the updates, will take a closer look at it using `0.17.0rc.3'

@mhdawson
Copy link
Author

mhdawson commented Jul 28, 2023

So updated to `0.17.0rc.3' and I pushed the builder to my local docker repostory and get a different error. I'm still a bit confused how it was working before, maybe the pull of the builder image meta data is new since the older version it worked with?

The new error seems to indicate it is still looking in the docker registry even though we are trying to test a local version.

       [restorer] Pulling builder image metadata for index.docker.io/library/testbuilder:latest...
        [restorer] ERROR: failed to pull builder image: failed to get remote image
        ERROR: failed to build: executing lifecycle: failed with status code: 1
        
        Unexpected error:
            <*fmt.wrapError | 0xc0003ea120>: 
            failed to pack build: exit status 1

@mhdawson
Copy link
Author

extensions perform run image switching (only, without extension)

We do that, so likely affected but don't think it looks like we get that far.

@mhdawson
Copy link
Author

(previous versions of pack and the lifecycle had bugs that resulted in silent failure to pull the builder image manifest).

I see that explains why things worked before.

@natalieparellano
Copy link
Member

natalieparellano commented Jul 28, 2023

The new error seems to indicate it is still looking in the docker registry even though we are trying to test a local version.

This is true. I'm actually surprised the build got as far as it did, because the pack client should enforce --pull-policy=always and try to re-pull the builder from a remote registry as part of this policy:

return fmt.Errorf("pull policy must be 'always' when builder contains image extensions")

@natalieparellano
Copy link
Member

I did some digging and I think I understand why the build did not fail earlier - pack when using --pull-policy=always (which the smoke tests seem to enforce) will still fall back to a daemon image if the remote image (index.docker.io/library/testbuilder:latest in this case) is not found.

When extending an image (either the builder image or the run image) we must be able to pull the remote image, because we need to provide the manifest to kaniko. For the builder image, you could spin up a local registry like we recommend in the extension docs.

For the run image, if you are NOT extending it (only switching to a new image) we are currently still requiring the remote image to be available, but I think this is a mistake - we don't need a manifest for kaniko, and only need to read "target" data (image config values) for which a daemon image would work just fine. I can make this change in the lifecycle and pack. This would avoid needing to push the run image to the local registry.

@natalieparellano
Copy link
Member

@mhdawson I managed to get the smoke tests passing again with a dev lifecycle and a dev pack... basically I commented out the stuff that installs pack and I tagged over buildpacksio/lifecycle:0.17.0-rc.4 in my daemon with a dev image lifecycle image. It wasn't pretty. Additionally I ran a local registry before running the tests, so the end of main() in scripts/smoke.sh looked like:

reg=$(docker run -d -p5000:5000 registry:2)
builder::create "localhost:5000/${name}"
docker push "localhost:5000/${name}" 
tests::run "localhost:5000/${name}"
docker stop $reg

I needed to add WithNetwork("host"). to line 61 of smoke/nodejs_test.go so that the test registry would be reachable.

@mhdawson
Copy link
Author

mhdawson commented Aug 3, 2023

@natalieparellano thanks for getting that to work. Just to confirm, the only changes that will be required once we have a version of pack with the newer lifecycle will be the change in nodejs_test.go on line 61 and then those changes in scripts/smoke.sh? If so that is good news and I'll queue up a PR that we can land once there is a newer pack.

Once I'm back from holiday (I'm out from Aug 7 through 11) I'll look at how to apply something similar to the integration testing for the extension (versus the builder) which uses occam. My guess is we might need some updates to occam to optionally use a registry to build/run images. @pacostas if you have some cycles before I'm back this might be something you can look at as well.

@jjbustamante
Copy link
Member

Hi @mhdawson.

Could you be able to test your set up with the new version of lifecycle 0.17.0 and the new pack's release candidate 0.30.0-rc2? Will be great if you can confirm that everything works as expected with these versions or if there is still something to be fix it on pack side.

@jjbustamante jjbustamante added status/in-progress Issue or PR that is currently in progress. and removed status/triage Issue or PR that requires contributor attention. labels Aug 14, 2023
@mhdawson
Copy link
Author

@jjbustamante thanks, just back from holidays last week. Will try to test it out early this week.

@mhdawson
Copy link
Author

@jjbustamante I checked it out and it seems to work ok. The PR to update the builder-ubi-base to use the newer lifecycle and pack versions is in paketo-community/builder-ubi-base#11.

Let me know if there was anything else I should have updated/tested.

@jjbustamante
Copy link
Member

@mhdawson sweet!

Thanks for testing this out, I think we are ok. We can leave this issue open until pack 0.30.0 is released (hopefully by the end of this week or the next one), after that, you can update your paketo PR to use the final pack version and then I will close this ticket!

Thanks a lot!!

@mhdawson
Copy link
Author

@jjbustamante thanks for your help :)

@jjbustamante jjbustamante added status/resolved Support issues that have been resolved. and removed status/in-progress Issue or PR that is currently in progress. labels Aug 17, 2023
@jjbustamante
Copy link
Member

Closing, pack 0.30.0 was released!

@jjbustamante jjbustamante added this to the 0.30.0 milestone Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/resolved Support issues that have been resolved. type/bug Issue that reports an unexpected behaviour.
Projects
None yet
Development

No branches or pull requests

4 participants