Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

asset support for unpacking a single docker layer #112

Merged
merged 1 commit into from
Jun 15, 2018
Merged

asset support for unpacking a single docker layer #112

merged 1 commit into from
Jun 15, 2018

Conversation

dexhorthy
Copy link
Member

What I Did

  • add support for unpacking a single docker layer to a target destination
---
assets:
  v1:
    - dockerlayer:
        dest: deb/docker/
        image: quay.io/replicated/docker-packages:ubuntu-1604-v1.12.3.20180417
        source: public
        layer: f7126e84abc96fbc8495c33052724fad48115829e86987adbf556474f0ead5c1

lifecycle:
  v1:
  - render: {}

How I Did it

  • add new asset DockerLayer, which extends DockerAsset
  • move some docker helper structs and interfaces into a new images package
  • Refactor planner.dockerStep into docker package as docker.Renderer so we can reuse it for dockerlayer
  • add dockerlayer.Unpacker struct that can unpack a docker layer, delegating to the docker.Renderer

The Unpacker does the following:

  • docker save the image to a tmp directory
  • untar the image.tar into a second tmp directory
  • untar the second tmp directory/<layer sha>/layer.tar into the asset's dest

Other stuff

  • we were missing a fail with an error on unsupported plan steps, had to refactor the planner.Build() to be able to return an error
  • some other random refactors and improvements

How to verify it

  • Run the example in examples/dockerlayer/packages.yml
  • write more assets that use dockerlayer

Description for the Changelog

Added support for dockerlayer assets, which

Picture of a Boat (not required but encouraged)

@dexhorthy dexhorthy requested a review from divolgin June 15, 2018 22:47
"gopkg.in/yaml.v2"
)

func TestDeserialize(t *testing.T) {
Copy link

Choose a reason for hiding this comment

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

Function TestDeserialize has 122 lines of code (exceeds 50 allowed). Consider refactoring.

Copy link
Member Author

Choose a reason for hiding this comment

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

/wontfix

}

func (u *Unpacker) save(
ctx context.Context,
Copy link

Choose a reason for hiding this comment

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

Method Unpacker.save has 5 arguments (exceeds 4 allowed). Consider refactoring.

}

func NewUnpacker(
logger log.Logger,
Copy link

Choose a reason for hiding this comment

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

Function NewUnpacker has 5 arguments (exceeds 4 allowed). Consider refactoring.

"github.com/stretchr/testify/require"
)

func TestUnpackLayer(t *testing.T) {
Copy link

Choose a reason for hiding this comment

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

Function TestUnpackLayer has 70 lines of code (exceeds 50 allowed). Consider refactoring.


// NewStep gets a new Renderer with the default impl
func NewStep(
logger log.Logger,
Copy link

Choose a reason for hiding this comment

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

Function NewStep has 5 arguments (exceeds 4 allowed). Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented Jun 15, 2018

Code Climate has analyzed commit 1eea0d4 and detected 11 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 9
Duplication 2

The test coverage on the diff in this pull request is 67.9% (50% is the threshold).

This pull request will bring the total coverage in the repository to 48.5% (-0.2% change).

View more on Code Climate.

@dexhorthy
Copy link
Member Author

I reviewed CC and they're all "this table test is too long"

@dexhorthy dexhorthy merged commit 3be0036 into replicatedhq:master Jun 15, 2018
@dexhorthy dexhorthy deleted the dex/ch11279/tic-dpvc branch June 19, 2018 16:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants