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

Create archive for the kicbase image #7766

Closed
afbjorklund opened this issue Apr 18, 2020 · 10 comments
Closed

Create archive for the kicbase image #7766

afbjorklund opened this issue Apr 18, 2020 · 10 comments
Labels
co/docker-driver Issues related to kubernetes in container co/podman-driver podman driver issues kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@afbjorklund
Copy link
Collaborator

afbjorklund commented Apr 18, 2020

(breakout from issue #7012, as a requirement)

In order to be able to show some progress while downloading the big "kicbase" image,
and for accessing it from environments with limited access to Google Container Registry -
we want to create a tarball with the image. This also allows loading it for all container runtimes,
the current github.com/google/go-containerregistry library only supports the docker daemon.

We want to treat it similar to the minikube.iso


As workaround, we could load the kicbase image from a tarball that we download ?

313M gcr.io/k8s-minikube/kicbase_v0.0.9
docker load -i kicbase_v0.0.9.tar # and similar functions, from the "cruntime" facade

That way, cache would also work with the non-daemon drivers (such as podman).

docker pull gcr.io/k8s-minikube/kicbase:v0.0.9
docker save gcr.io/k8s-minikube/kicbase:v0.0.9 > kicbase_v0.0.9.tar

This would also make it available in china #7447 and without dockerhub #7316

There is a related problem, that the "digest" is not loaded from these alternative formats.

The suggestion is that we verify the tarball with a separate sha256 checksum file instead.

@afbjorklund afbjorklund added co/docker-driver Issues related to kubernetes in container co/podman-driver podman driver issues kind/feature Categorizes issue or PR as related to a new feature. labels Apr 18, 2020
@afbjorklund afbjorklund changed the title Create tarball for the kicbase image Create archive for the kicbase image Apr 18, 2020
@afbjorklund afbjorklund added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Apr 18, 2020
@medyagh
Copy link
Member

medyagh commented Apr 18, 2020

I like the idea ! we could also put the image tar in the releases attachment in github.
ny metrics of how much additional tax we will have to pay in seconds for load and save?

@afbjorklund
Copy link
Collaborator Author

ny metrics of how much additional tax we will have to pay in seconds for load and save?

No, but that could be a good investigation for someone. It's a one time thing, though...
For most users, downloading a couple of hundred megabytes take longer than unpacking.

As opposed to the preload, I don't think there will be any major difference in the time.
The goal here is to enable showing a progress bar, and allow using more container runtimes.

It could have been fixed in the library instead, but development seems to have stalled ?

i.e. if github.com/google/go-containerregistry supported progress bars and non-daemon

the first feature (progress) has been open for months

and the second feature (daemon) was even scoped out

@afbjorklund
Copy link
Collaborator Author

@medyagh : you need to be willing to drop the "digest" demand, though. only works with pull.

So the "tag" would have to be enough, otherwise we still need to invoke docker pull and friends.

docker inspect

[
    {
        "Id": "sha256:819f427ce27572658c87e41384c2e67bae040a5942dd692233c19cad218e36b3",
        "RepoTags": [
            "gcr.io/k8s-minikube/kicbase:v0.0.9"
        ],
        "RepoDigests": [
            "gcr.io/k8s-minikube/kicbase@sha256:82a826cc03c3e59ead5969b8020ca138de98f366c1907293df91fc57205dbb53"
        ],
        "Parent": "",

manifest.json

[
  {
    "Config": "819f427ce27572658c87e41384c2e67bae040a5942dd692233c19cad218e36b3.json",
    "RepoTags": [
      "gcr.io/k8s-minikube/kicbase:v0.0.9"
    ],
    "Layers": [
      "dbd135ce57669d2e7aa83cba9fff6022e8420a2b49225a2f0f53b93c76d4fff2/layer.tar",
      "6b3e1727e0a4b0ba3108910866e2dab4cc4e4627bed9371e10af0aa544aadc9d/layer.tar",
      "fd9639bd8644e241276704b3d11e67f1a54fc26021f4da0d58be0bc0885cab0d/layer.tar",
      "9879b0d9f44493430238ae62eea61b07053f37e6a3dbdd95dba6706708629f19/layer.tar",
      "ea6e2e94183eaa37592110c0c5bf947c0795bc37164b3cc8c1cbde4fec542778/layer.tar",
      "d649a18976035975a4bd026b7a7ed74ef77518f541bd6c48546c4651319fffff/layer.tar",
      "88e29628b7bdd61dd1aa776ce7465cbc78b7e776f6bd447f09e69187528f664e/layer.tar",
      "30b13476b800565ef5e742a0b2a05efb0cf7398ec8d186f14044ed4881c889e2/layer.tar",
      "dd14ea1ccea15515111486e7db3b7c6d5cd6581c0df9f490160d67ebdada829f/layer.tar",
      "559f0bd36115dbfabf6ad7a78a4b02dad5b4b8cca3bc6d41e23ba971dbfb0824/layer.tar",
      "6038e99d2aa2e62d0f4e7b98e4f3f2d74524e9005989926805ac78a2c17d0ca6/layer.tar",
      "a03ba59b21e929bdc004fc983538bef86b69be9e8447d7d62b5c447c28645e74/layer.tar",
      "522092459c643e196538679fd56997996edc5923ce91aaea99779715a5a222be/layer.tar",
      "b171cf54e5531e8583dbda26dd837eaee008d1deaecee4ce7a2aed7c7bb2ff4c/layer.tar",
      "b72b2345bd54b85a4ad013bd96634121ec541c524c5033dde8576a5274930653/layer.tar",
      "f58613cc563f946f41e43b72b0eddca96201eff631d1255d423df1761ed948cb/layer.tar",
      "39977a33ae11a44783208e104116929cbb8a092137e73db87f3d8e565a59d1c0/layer.tar",
      "550bf434a19fc5f2f52d451db86e199eadf702033fbe1f50ebe952fb9a2ec001/layer.tar",
      "2588948d618c205373cf0f7173ca515efad2d961a31b481d0085fe818464e040/layer.tar",
      "8e4bfd566a801605e1a970a176f738b26e0df75b8250dd5567e30d83ac59001c/layer.tar",
      "169c18c122a2aa8315cb7c1a78bd344e1a296f2668004940d574965cbebe2aba/layer.tar"
    ]
  }
]

See moby/moby#22011

As you know, it doesn't really work with the go-containerregistry go library now either...

So currently we just import the layers from the download, and then pull the manifest.

See google/go-containerregistry#540

@afbjorklund
Copy link
Collaborator Author

afbjorklund commented Apr 19, 2020

And now we have this wonderful workaround about it:

const (
	// Version is the current version of kic
	Version = "v0.0.9"
	// SHA of the kic base image
	baseImageSHA = "82a826cc03c3e59ead5969b8020ca138de98f366c1907293df91fc57205dbb53"
)

var (
	// BaseImage is the base image is used to spin up kic containers. it uses same base-image as kind.
	BaseImage = fmt.Sprintf("gcr.io/k8s-minikube/kicbase:%s@sha256:%s", Version, baseImageSHA)
)
pkg/minikube/registry/drvs/docker/docker.go:            ImageDigest:       kic.BaseImage,
pkg/minikube/registry/drvs/podman/podman.go:            ImageDigest:   strings.Split(kic.BaseImage, "@")[0], // for podman does not support docker images references with both a tag and digest.

That is, we just hack out the digest for non-docker anyway.

The current name "SHA" is somewhat none-descript ? Yes, is is a SHA-256 checksum.
But of what ? So better use the name "digest" - but then again we probably have to drop it.

The tarball will have a checksum of the compressed archive. Mostly for verifying download.
Like the files we provide for minikube itself, but then never use for anything (not in the docs)

 curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64
curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64.sha256
openssl sha256 minikube-linux-amd64 | awk '{print $2}'  > minikube-linux-amd64.checksum
cmp minikube-linux-amd64.sha256 minikube-linux-amd64.checksum
 sudo install minikube-linux-amd64 /usr/local/bin/minikube

Except that this one will actually be used by the download library, of course (like the iso's)

@priyawadhwa
Copy link

I spent the day trying to:

  1. Load the kic base image from a tarball into the docker daemon
  2. Make sure the image in the daemon somehow has the digest included so that we can run kic

Step 1 failed with an odd panic in go-containerregistry:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x5209183]

goroutine 78 [running]:
github.com/google/go-containerregistry/pkg/v1/tarball.MultiRefWrite(0xc00008ae68, 0x59e7ec0, 0xc0001601f0, 0x0, 0x0)
	/Users/priyawadhwa/go/pkg/mod/github.com/afbjorklund/[email protected]/pkg/v1/tarball/write.go:97 +0x2a3
github.com/google/go-containerregistry/pkg/v1/tarball.Write(...)
	/Users/priyawadhwa/go/pkg/mod/github.com/afbjorklund/[email protected]/pkg/v1/tarball/write.go:66
github.com/google/go-containerregistry/pkg/v1/daemon.Write.func1(0xc0001601f0, 0x5a37700, 0xc00013c550, 0x0, 0x0)
	/Users/priyawadhwa/go/pkg/mod/github.com/afbjorklund/[email protected]/pkg/v1/daemon/write.go:65 +0x101
created by github.com/google/go-containerregistry/pkg/v1/daemon.Write
	/Users/priyawadhwa/go/pkg/mod/github.com/afbjorklund/[email protected]/pkg/v1/daemon/write.go:64 +0x1ee

which I think is happening because the tarball package takes in type *name.Tag while all the other packages take in name.Tag code here

Not really sure if there's a fix for this, but even if there is, I don't see an easy way for kic to work offline if we can't pull the kicbase manifest (thus providing the image with a digest).

I think @afbjorklund makes a good point and we should drop the digest when running the kicbase image. We can still verify it, but instead of running

gcr.io/k8s-minikube/kicbase:v0.0.10@sha256:f58e0c4662bac8a9b5dda7984b185bad8502ade5d9fa364bf2755d636ab51438

I think we should just run

gcr.io/k8s-minikube/kicbase:v0.0.10

which should make offline docker driver possible. WDYT @afbjorklund @medyagh ?

@afbjorklund
Copy link
Collaborator Author

afbjorklund commented Jun 6, 2020

I spent the day trying to:

Load the kic base image from a tarball into the docker daemon
Make sure the image in the daemon somehow has the digest included so that we can run kic

Step 1 failed with an odd panic in go-containerregistry:

Ouch, wonder if that is something that I introduced in patch ?

Hopefully not, but anyway I rebased the code now (in PR #8397)

Also saw that support for progress bars had been added now:

google/go-containerregistry@984e0aa

@medyagh medyagh added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Jun 23, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 21, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 21, 2020
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
co/docker-driver Issues related to kubernetes in container co/podman-driver podman driver issues kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

No branches or pull requests

5 participants