Add flag to export image to OCI layout format - (Experimental) #1314

jjbustamante · 2021-10-26T19:28:30Z

Summary

Eric Hripko worked a few months ago on integrating Buildkit with Lifecycle (watch demo here), after that work some ideas on improving lifecycle were designed, one idea was to add a new flag to export the container image into OCI format.

This PR adds the experimental feature in Pack to invoke the lifecycle supporting this new flag for exporting the image.

Output

A new flag -oci-dir was added, this new flag is a string and the argument specifies the path in the local filesystem where the OCI image must be exported

The following image shows the arguments during the exporting phase

We can notice three things here:

layout flag is sent to the lifecycle
CNB_LAYOUT_DIR environment variable is also sent to lifecycle
We can notice a new volume mounted in the local filesystem and pointing to the directory specified in the environment variable CNB_LAYOUT_DIR is also created

Finally, the following image shows the final output. A new folder /oci is created and inside this folder, the image in OCI format is saved

Before

The exporting flag didn't exist

After

A new flag --oci-dir was added

Documentation

Should this change be documented?
- Yes, see #___
- [x ] No

A couple things:

Perhaps you're not already familiar with the os.MkdirAll stdlib function, which makes a directory if it doesn't it exists. It could simplify some of the code written here.

Perhaps we can always make an oci/ directory, even if the user provided a path. e.g. filepath.Join(ociPath, "oci"). Up to you but I think it's a pretty low stakes move and, again, it could simplify some of the code written here.

thanks the hint @aemengo I just did some changes in the following commit to simplify the code based on the feedback

aemengo · 2021-10-28T02:21:31Z

internal/build/lifecycle_execution.go

+	if l.opts.OCIPath != "" {
+		flagsOpt = WithFlags("-layout")
+	}
+	_, layoutOpt, layoutEnv := l.configureLayout([]string{})
+


What does the -layout flag do in the analyze phase? If it doesn't yet have a purpose, I'd rather that it would be kept out.

Well, during analyzing it is used to mount the previous image reading from the OCI layout format, for example, you ran the command and export your image to /oci folder in your local, if you run again the build, then the /oci previously generated is mounted and analyze do its job

aemengo · 2021-10-28T02:25:29Z

internal/build/lifecycle_execution.go

@@ -553,3 +565,15 @@ func addTags(flags, additionalTags []string) []string {
 	}
 	return flags
 }
+
+func (l *LifecycleExecution) configureLayout(flags []string) ([]string, PhaseConfigProviderOperation, PhaseConfigProviderOperation) {


Just my opinion here, but I'd rather not have this function. All the flags and PhaseConfigProviderOperation construction that we have is definitely ugly and overdue for a refactor. But having this function here, that I have to jump into to see what it's doing and what's being returned, doesn't help much.

I agreed that the code it's a little ugly :) , but I removed the function, just because it is making it hard to read the code.

I liked the idea though.

Please take a look at my commit and see if you like my interpretation of your idea.

Sweet!!! That's a more elegant approach!!! hahahah Thanks!

aemengo · 2021-10-28T02:28:35Z

internal/build/lifecycle_execution.go

+
+	if l.opts.OCIPath != "" {
+		flags = append(flags, "-layout")
+		layoutOpt = WithBinds(fmt.Sprintf("%s:%s:%s", l.opts.OCIPath, l.mountPaths.ociDir(), "rw"))


The downside of this WithBinds is that it wouldn't work for a remote docker daemon. If that matters, another option can be to copy out the directory before the container is stopped

See some sample code.

Oh! but you have that function in your interactive branch any chance to get that function merged into main to be available to use it?. I tried to find something similar but then I decided for WithBinds but thanks for feedback.

It would probably take longer get the function merged independently. 😅 You could copy-n-paste it into this PR. I'd be happy to pair with you on it, if needed.

@aemengo I did some changes, I took the function you created in the interactive branch and removed the WithBinds. As I mentioned, I was having some issues with the unit tests, but following your suggestions, I added a new method in the export, could you just take a look? it's this line

I like it much better. 👍🏾 from me.

I was inspired though and took it a little further. Please take a look at my commit and see if how you feel about it.

Yes, I can see you went into the zone ... 😄

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

…xporting path then we assumed the feature must be enable, also the flag is hidden under experimentals flags Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

…xporting path then we assumed the feature must be enable, also the flag is hidden under experimentals flags Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

jromero · 2022-03-22T20:41:39Z

Whatever happened to this? Should this be draft or do we still want to push it out?

jjbustamante · 2022-03-22T22:45:03Z

Whatever happened to this? Should this be draft or do we still want to push it out?

I think the best is to convert the PR to Draft

jjbustamante · 2023-01-31T23:26:49Z

Closing this PR on behalf of 1596

jjbustamante requested a review from a team as a code owner October 26, 2021 19:28

github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Oct 26, 2021

github-actions bot added this to the 0.22.0 milestone Oct 26, 2021

aemengo suggested changes Oct 28, 2021

View reviewed changes

jromero modified the milestones: 0.22.0, 0.23.0 Nov 2, 2021

jjbustamante added 9 commits November 3, 2021 18:11

WIP - adding -layout flag to export image to OCI format

0f6ac6b

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - adding capability to export the oci image to user's file system

b9a1b38

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - merging OCI Path and layout flag in one, if we define the oci e…

aa4bea7

…xporting path then we assumed the feature must be enable, also the flag is hidden under experimentals flags Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - adding -layout flag to export image to OCI format

fcc4b8b

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - adding capability to export the oci image to user's file system

6184e23

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - merging OCI Path and layout flag in one, if we define the oci e…

32e0af3

…xporting path then we assumed the feature must be enable, also the flag is hidden under experimentals flags Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - removing unused variable after rebasing against main

510e289

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - adding unit tests for oci-dir flag

56a3378

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

WIP - formatting issues and new assert

d8c043b

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

jjbustamante force-pushed the feature/oci-layout branch 2 times, most recently from f6ef239 to c61754f Compare November 4, 2021 01:10

WIP - OCI Layout feature

e75288b

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

jjbustamante force-pushed the feature/oci-layout branch from c61754f to e75288b Compare November 4, 2021 01:19

samj1912 mentioned this pull request Dec 5, 2021

Idea: Add support for OCI layout / deprecate daemon support buildpacks/rfcs#197

Closed

dfreilich modified the milestones: 0.23.0, 0.24.0 Dec 7, 2021

dfreilich modified the milestones: 0.24.0, 0.25.0 Feb 21, 2022

jromero marked this pull request as draft March 29, 2022 18:36

dfreilich modified the milestones: 0.25.0, 0.26.0 Apr 15, 2022

jromero removed this from the 0.26.0 milestone Apr 20, 2022

jromero added the experimental Issue or PR refers to an experimental feature. label Aug 24, 2022

jjbustamante closed this Jan 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flag to export image to OCI layout format - (Experimental) #1314

Add flag to export image to OCI layout format - (Experimental) #1314

jjbustamante commented Oct 26, 2021 •

edited

Loading

aemengo left a comment •

edited

Loading

aemengo Oct 28, 2021

jjbustamante Oct 28, 2021

aemengo Oct 28, 2021

jjbustamante Oct 28, 2021

aemengo Oct 28, 2021

jjbustamante Nov 4, 2021

aemengo Nov 4, 2021

jjbustamante Nov 4, 2021

aemengo Oct 28, 2021

jjbustamante Oct 29, 2021

aemengo Oct 29, 2021

jjbustamante Nov 4, 2021

aemengo Nov 4, 2021

jjbustamante Nov 4, 2021

jromero commented Mar 22, 2022

jjbustamante commented Mar 22, 2022

jjbustamante commented Jan 31, 2023

               	return all, nil
               }
+              func (c *Client) processOCIPath(ociPath string) (string, error) {

Add flag to export image to OCI layout format - (Experimental) #1314

Add flag to export image to OCI layout format - (Experimental) #1314

Conversation

jjbustamante commented Oct 26, 2021 • edited Loading

Summary

Output

Before

After

Documentation

Related

aemengo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jromero commented Mar 22, 2022

jjbustamante commented Mar 22, 2022

jjbustamante commented Jan 31, 2023

jjbustamante commented Oct 26, 2021 •

edited

Loading

aemengo left a comment •

edited

Loading