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

Extension package #1661

Merged
merged 22 commits into from
Mar 31, 2023
Merged

Extension package #1661

merged 22 commits into from
Mar 31, 2023

Conversation

itsdarshankumar
Copy link
Contributor

@itsdarshankumar itsdarshankumar commented Mar 6, 2023

Summary

Implements the functionality for pack extension package command

Output

image

Before

No command exists

After

image

package-extension

Related

Resolves #1635

@itsdarshankumar itsdarshankumar requested review from a team as code owners March 6, 2023 21:32
@itsdarshankumar itsdarshankumar marked this pull request as draft March 6, 2023 21:32
@github-actions github-actions bot added this to the 0.29.0 milestone Mar 6, 2023
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Mar 6, 2023
@itsdarshankumar itsdarshankumar force-pushed the extension-package branch 2 times, most recently from 271f4d8 to ccb01e8 Compare March 6, 2023 23:01
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
itsdarshankumar and others added 3 commits March 7, 2023 04:47
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
@itsdarshankumar itsdarshankumar marked this pull request as ready for review March 7, 2023 00:22
@itsdarshankumar itsdarshankumar force-pushed the extension-package branch 2 times, most recently from 96a5c84 to e9a6172 Compare March 7, 2023 04:37
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #1661 (45eb473) into main (f1cde32) will decrease coverage by 0.72%.
The diff coverage is 41.25%.

❗ Current head 45eb473 differs from pull request most recent head 9525a10. Consider uploading reports for the commit 9525a10 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1661      +/-   ##
==========================================
- Coverage   81.13%   80.41%   -0.72%     
==========================================
  Files         165      165              
  Lines       10885    11037     +152     
==========================================
+ Hits         8831     8874      +43     
- Misses       1525     1627     +102     
- Partials      529      536       +7     
Flag Coverage Δ
os_linux ?
os_macos ?
os_windows 80.41% <41.25%> (+0.08%) ⬆️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

itsdarshankumar and others added 4 commits March 13, 2023 23:41
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
@natalieparellano
Copy link
Member

@itsdarshankumar apologies for the slow reply (I've been out). I'll take a look soon.

Copy link
Member

@natalieparellano natalieparellano left a comment

Choose a reason for hiding this comment

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

As always, amazing work @itsdarshankumar! Thank you so much for the PR!

@buildpacks/platform-maintainers I think this is ready for your review

@pacostas
Copy link

I tested the functionality and looks like the pack extension package test-package does not generate anything despite the successful message Successfully created package test-package. I've also set the extension.toml to have below values.

extension.id = "test-packagse"
extension.version = "0.1"

Is there any other configuration necessary? Also, should the extension.toml be aligned with the toml format?

@itsdarshankumar
Copy link
Contributor Author

I tested the functionality and looks like the pack extension package test-package does not generate anything despite the successful message Successfully created package test-package. I've also set the extension.toml to have below values.

extension.id = "test-packagse"
extension.version = "0.1"

Is there any other configuration necessary? Also, should the extension.toml be aligned with the toml format?

can you send me your whole toml file and bin, I've tested the functionality from samples/ repo.

@natalieparellano
Copy link
Member

Tested this out locally and it's working for me. I think there could be a couple things going on here:

  • The config file may not be valid, in the comment above I see id and version but not uri which is necessary for pack to find the extension bits.
  • Without --format=file the default location for the created package will be the docker daemon; if you run docker inspect <generated package name> it should be there.

My config:

[extension]
uri = "/Users/narellano/workspace/samples/extensions/tree"

Generating an image:

$  ~/workspace/pack/out/pack extension package test-ext --config ./package.toml
Successfully created package test-ext

$  docker inspect test-ext
[
    {
        "Id": "sha256:66ff587305515f9354e690710d577e198dd4f1f40e41c64745cf3d3b775a3ea8",
        "RepoTags": [
            "test-ext:latest"
        ],
...

Generating a file:

$  ~/workspace/pack/out/pack extension package test-ext --config ./package.toml --format=file
Successfully created package test-ext.cnb

$  ls
bin		extension.toml	package.toml	test-ext.cnb

@mhdawson
Copy link

mhdawson commented Mar 24, 2023

Looking at the issue @pacostas was having, with this PR - https://github.com/nodeshift/ubi-nodejs-extension/pull/33

I can use jam to create a tgz (although not completely sure it is used by the pack extension invocation) and then create the cnb using the pack build from this branch
using:

../pack/out/pack extension package test.cnb --format=file

However if I try to use the resuting .cnb file as follows:

[[extensions]]
    id = "redhat-runtimes/nodejs"
    version = "0.0.1"
#    uri = "file:///home/user1/paketo/ubi-nodejs-extension"
    uri = "file:///home/user1/paketo/ubi-nodejs-extension/test.cnb"

I get this SIGSEGV

[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0xee1389]

goroutine 1 [running]:
github.com/buildpacks/pack/pkg/client.validateModule({0x11e4b08, 0x9}, {0x0?, 0x0?}, {0xc00038ee40, 0x37}, {0xc0003aaf60, 0x16}, {0xc0001655c0, 0x5})
	github.com/buildpacks/pack/pkg/client/create_builder.go:294 +0x69
github.com/buildpacks/pack/pkg/client.(*Client).addConfig(_, {_, _}, {_, _}, {{{0xc0003aaf60, 0x16}, {0x0, 0x0}, {0xc0001655c0, ...}, ...}, ...}, ...)
	github.com/buildpacks/pack/pkg/client/create_builder.go:253 +0x4d1
github.com/buildpacks/pack/pkg/client.(*Client).addExtensionsToBuilder(_, {_, _}, {{0xc0001f58f0, 0x1f}, {0x7ffd4c93b2c7, 0x1b}, {{0xc000054900, 0x7d}, {0xc000383800, ...}, ...}, ...}, ...)
	github.com/buildpacks/pack/pkg/client/create_builder.go:225 +0x167
github.com/buildpacks/pack/pkg/client.(*Client).CreateBuilder(_, {_, _}, {{0xc0001f58f0, 0x1f}, {0x7ffd4c93b2c7, 0x1b}, {{0xc000054900, 0x7d}, {0xc000383800, ...}, ...}, ...})
	github.com/buildpacks/pack/pkg/client/create_builder.go:60 +0x198
github.com/buildpacks/pack/internal/commands.BuilderCreate.func1(0xc00037ac00, {0xc00031d140, 0x1, 0x0?})
	github.com/buildpacks/pack/internal/commands/builder_create.go:75 +0x563
github.com/buildpacks/pack/internal/commands.logError.func1(0xc00037ac00?, {0xc00031d140?, 0x3?, 0x3?})
	github.com/buildpacks/pack/internal/commands/commands.go:58 +0x43
github.com/spf13/cobra.(*Command).execute(0xc00037ac00, {0xc00031d110, 0x3, 0x3})
	github.com/spf13/cobra@v1.6.1/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc00037a000)
	github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/cobra@v1.6.1/command.go:968
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	github.com/spf13/cobra@v1.6.1/command.go:961
main.main()
	github.com/buildpacks/pack/cmd/pack/main.go:26 +0x111

Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
@itsdarshankumar
Copy link
Contributor Author

Looking at the issue @pacostas was having, with this PR - nodeshift/ubi-nodejs-extension#33

I can use jam to create a tgz (although not completely sure it is used by the pack extension invocation) and then create the cnb using the pack build from this branch using:

../pack/out/pack extension package test.cnb --format=file

However if I try to use the resuting .cnb file as follows:

[[extensions]]
    id = "redhat-runtimes/nodejs"
    version = "0.0.1"
#    uri = "file:///home/user1/paketo/ubi-nodejs-extension"
    uri = "file:///home/user1/paketo/ubi-nodejs-extension/test.cnb"

I get this SIGSEGV

[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0xee1389]

goroutine 1 [running]:
github.com/buildpacks/pack/pkg/client.validateModule({0x11e4b08, 0x9}, {0x0?, 0x0?}, {0xc00038ee40, 0x37}, {0xc0003aaf60, 0x16}, {0xc0001655c0, 0x5})
	github.com/buildpacks/pack/pkg/client/create_builder.go:294 +0x69
github.com/buildpacks/pack/pkg/client.(*Client).addConfig(_, {_, _}, {_, _}, {{{0xc0003aaf60, 0x16}, {0x0, 0x0}, {0xc0001655c0, ...}, ...}, ...}, ...)
	github.com/buildpacks/pack/pkg/client/create_builder.go:253 +0x4d1
github.com/buildpacks/pack/pkg/client.(*Client).addExtensionsToBuilder(_, {_, _}, {{0xc0001f58f0, 0x1f}, {0x7ffd4c93b2c7, 0x1b}, {{0xc000054900, 0x7d}, {0xc000383800, ...}, ...}, ...}, ...)
	github.com/buildpacks/pack/pkg/client/create_builder.go:225 +0x167
github.com/buildpacks/pack/pkg/client.(*Client).CreateBuilder(_, {_, _}, {{0xc0001f58f0, 0x1f}, {0x7ffd4c93b2c7, 0x1b}, {{0xc000054900, 0x7d}, {0xc000383800, ...}, ...}, ...})
	github.com/buildpacks/pack/pkg/client/create_builder.go:60 +0x198
github.com/buildpacks/pack/internal/commands.BuilderCreate.func1(0xc00037ac00, {0xc00031d140, 0x1, 0x0?})
	github.com/buildpacks/pack/internal/commands/builder_create.go:75 +0x563
github.com/buildpacks/pack/internal/commands.logError.func1(0xc00037ac00?, {0xc00031d140?, 0x3?, 0x3?})
	github.com/buildpacks/pack/internal/commands/commands.go:58 +0x43
github.com/spf13/cobra.(*Command).execute(0xc00037ac00, {0xc00031d110, 0x3, 0x3})
	github.com/spf13/cobra@v1.6.1/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc00037a000)
	github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/cobra@v1.6.1/command.go:968
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	github.com/spf13/cobra@v1.6.1/command.go:961
main.main()
	github.com/buildpacks/pack/cmd/pack/main.go:26 +0x111

can you please send the toml file of your extension through which you are packaging it

@mhdawson
Copy link

mhdawson commented Mar 24, 2023

@itsdarshankumar thanks for taking a look. It is this one: https://github.com/nodeshift/ubi-nodejs-extension/blob/main/extension.toml

If you want to try out the same as what I did you could (changing the path to where you have build the updated pack to match in the below):

  1. clone the repo https://github.com/nodeshift/ubi-nodejs-extension.
  2. cd ubi-nodejs-extension
  3. run ../../pack config experimental true
  4. run ../../pack/out/pack extension package test.cnb --format=file
  5. add the following to a builder.toml file (updating so have the paths that match where you build test.cnb)
    description = "Sample builder that uses ubi Node.js extension to support Node.js apps"
    
    [[buildpacks]]
      uri = "docker://gcr.io/paketo-buildpacks/nodejs:1.4.0"
      version = "1.4.0"
    
    [lifecycle]
      version = "0.15.3"
    
    [[order]]
      [[order.group]]
        id = "paketo-buildpacks/nodejs"
        version = "1.4.0"
    
    [[extensions]]
      id = "redhat-runtimes/nodejs"
      version = "0.0.1"
      uri = "file:///home/user1/paketo/testit/ubi-nodejs-extension/test.cnb"
    
    [[order-extensions]]
      [[order-extensions.group]]
        id = "redhat-runtimes/nodejs"
        version = "0.0.1"
    
    [stack]
      id = "ubi8-paketo"
      build-image = "quay.io/midawson/ubi8-paketo-build"
      run-image = "quay.io/midawson/ubi8-paketo-run"
    
  6. runpack config experimental trueu
    ../pack/out/pack builder create localhost:5000/test-builder --config ./builder.toml
    

@natalieparellano
Copy link
Member

@mhdawson thanks for reporting - I can reproduce the issue. The generated package is actually fine, the SIGSEGV is due to a bug in pack builder create where downloads for extensions are unimplemented and we're returning nil here instead of a helpful error.

Once this PR is merged and we have packages for extensions we'll be able to add the implementation in pack builder create - here is the issue to track the progress.

@dfreilich dfreilich modified the milestones: 0.29.0, 0.30.0 Mar 26, 2023
@pacostas
Copy link

pacostas commented Mar 27, 2023

Tested this out locally and it's working for me. I think there could be a couple things going on here:

  • The config file may not be valid, in the comment above I see id and version but not uri which is necessary for pack to find the extension bits.
  • Without --format=file the default location for the created package will be the docker daemon; if you run docker inspect <generated package name> it should be there.

My config:

[extension]
uri = "/Users/narellano/workspace/samples/extensions/tree"

Generating an image:

$  ~/workspace/pack/out/pack extension package test-ext --config ./package.toml
Successfully created package test-ext

$  docker inspect test-ext
[
    {
        "Id": "sha256:66ff587305515f9354e690710d577e198dd4f1f40e41c64745cf3d3b775a3ea8",
        "RepoTags": [
            "test-ext:latest"
        ],
...

Generating a file:

$  ~/workspace/pack/out/pack extension package test-ext --config ./package.toml --format=file
Successfully created package test-ext.cnb

$  ls
bin		extension.toml	package.toml	test-ext.cnb

Thank you, eventually I missed the --format file argument. Everything works fine.

@mhdawson
Copy link

@natalieparellano thanks for taking the time to recreate and confirming :)

@natalieparellano
Copy link
Member

@dfreilich @jkutner this one is ready for your review :)

Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
Signed-off-by: Darshan Kumar <itsdarshankumar@gmail.com>
@jkutner jkutner merged commit 52127f7 into buildpacks:main Mar 31, 2023
@itsdarshankumar
Copy link
Contributor Author

hello @pacostas @mhdawson!!!, now you can try your work again as #1684 is now complete.

@mhdawson
Copy link

mhdawson commented May 29, 2023

@itsdarshankumar, thanks for the heads up, I confirmed that the crash reported in #1661 (comment) did not recreate follow the steps I'd listed in #1661 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pack should support pack extension package
6 participants