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

WAIT/END doesn't export to remote cache #2178

Open
alexcb opened this issue Sep 12, 2022 · 4 comments
Open

WAIT/END doesn't export to remote cache #2178

alexcb opened this issue Sep 12, 2022 · 4 comments
Labels
type:bug Something isn't working

Comments

@alexcb
Copy link
Collaborator

alexcb commented Sep 12, 2022

consider an Earthfile like:

VERSION --wait-block 0.6

deps:
   FROM ubuntu:22.04
   RUN install test dependencies…
   SAVE IMAGE --cache-hint

ci:
    BUILD +deps
    FROM +deps
    COPY . ./
    RUN --no-cache pytest --junitxml=report.xml ; echo $? > exit-code
    WAIT
        SAVE ARTIFACT report.xml AS LOCAL report.xml
    END
    RUN exit $(cat exit-code)

The SAVE IMAGE --cache-hint doesn't get triggered by the END, even though we wait for SAVE ARTIFACT which depends on the previous llb states being executed.

based on a post by @glehmann in #988 (comment)_

@alexcb
Copy link
Collaborator Author

alexcb commented Dec 7, 2022

I suspect we will need to extend

type ExportRequest struct {
	Refs     map[string]Reference
	Metadata map[string][]byte
}

to support referencing

type SolveOpt struct {

	CacheExports          []CacheOptionsEntry
	CacheImports          []CacheOptionsEntry

@alexcb
Copy link
Collaborator Author

alexcb commented Jan 7, 2023

debugging notes:

I've been running:

./test.sh --build-arg DOCKERHUB_AUTH=true --build-arg DOCKERHUB_USER_SECRET=+secrets/earthly-technologies/dockerhub-mirror/user --build-arg DOCKERHUB_TOKEN_SECRET=+secrets/earthly-technologies/dockerhub-mirror/pass --build-arg DOCKERHUB_MIRROR=registry-1.docker.io.mirror.corp.earthly.dev

under tests/remote-cache.

when I run curl localhost:5000/v2/test1/manifests/latest on VERSION 0.6, I get:

...
   "history": [
      {
         "v1Compatibility": "{\"architecture\":\"amd64\",\"config\":{\"Env\":[\"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\"],\"Cmd\":[\"/bin/sh\"]},\"created\":\"2023-01-07T00:58:06.750358406Z\",\"id\":\"19fb4b29241a864424d6550f643bc6d71c9663992c9a6063f56c9db839d04066\",\"moby.buildkit.cache.v0\":\"W3sibGF5ZXJzIjpbeyJsYXllciI6MSwiY3JlYXRlZEF0IjoiMjAyMy0wMS0wN1QwMDo1ODowMS4xNTQ0OTczOTFaIn1dLCJkaWdlc3QiOiJzaGEyNTY6NGVmNTJjMmI1MTNmODFmODQ5YTY3MzQzNDAzMDlmNTJjNjkxNmI1OGE3MTU2OTgxNDIxODM5Y2QzZGE3N2U5NCIsImlucHV0cyI6W1t7ImxpbmsiOjN9LHsibGluayI6NX1dXX0seyJsYXllcnMiOlt7ImxheWVyIjozLCJjcmVhdGVkQXQiOiIyMDIzLTAxLTA3VDAwOjU4OjA2Ljc1MDQ0NjIyWiJ9XSwiZGlnZXN0Ijoic2hhMjU2OjU3MDYzMzlkMjllODE4ZTY3NmVhMGUwYTFiNTNjODkyYTE1OTA3YmY2YTI3YTNlNzY1MzA4ZDIxMGI4MGNkMzYiLCJpbnB1dHMiOltbeyJsaW5rIjoyfV1dfSx7ImxheWVycyI6W3sibGF5ZXIiOjIsImNyZWF0ZWRBdCI6IjIwMjMtMDEtMDdUMDA6NTg6MDYuNjkwNzY1NzI4WiJ9XSwiZGlnZXN0Ijoic2hhMjU2OjZhM2NjZGFkZTNmMzRhYTY4NTAwMWZlYTQzZWExMWZjYTdlY2I0ZTY1YjI3YTJjZTljNDkxZWNkNjA4NzlhMTAiLCJpbnB1dHMiOltbeyJsaW5rIjowfV0sW3sibGluayI6NH1dXX0seyJkaWdlc3QiOiJzaGEyNTY6YTNhYzNmOTgyY2M0OTZjY2ZjNDE2NTIwNzFlYWY1ZTk3MWRiZTAxMWM1MDdhMzY2YmZmM2NmYzA5NWY2NjNhMSJ9LHsiZGlnZXN0Ijoic2hhMjU2OmRiNmIzN2I1OWU2NDBmNzNmY2U0MTRjNzE3ZDBjOWQzOGUwYjRkYzJhMWNkNGQ2M2Q4MzNiZmZhMTViYmI1YTAifSx7ImRpZ2VzdCI6InNoYTI1NjpmZDQ2ODMwMmUwNGRiMDUyMDkwMGUwMTEyNzUxYzlmNTI3M2FmMTdjNTMyOWI0MTZjODRiODcxOTkxMTE2OGUwIn1d\",\"os\":\"linux\",\"parent\":\"1bdf06011c0464980b327af4f9e9eaecc55fb119420a8c41def5ddc484cbce89\"}"
      },
...

note that it has a moby.buildkit.cache.v0 key.

where as in the VERSION 0.7, it doesnt:

...
   "history": [
      {
         "v1Compatibility": "{\"architecture\":\"amd64\",\"config\":{\"Env\":[\"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\"],\"Cmd\":[\"/bin/sh\"]},\"created\":\"2023-01-07T00:50:00.557864164Z\",\"id\":\"453fb18a0ce2c50b432c555778b29af92858a7849808a70539e71e7c98ed6c06\",\"os\":\"linux\",\"parent\":\"15f3d0f944a0f5d88a1aba8e20022826141259a40ace4b1fd29c2622ddc30075\"}"
      },
...

@alexcb
Copy link
Collaborator Author

alexcb commented Jan 7, 2023

The above reflect the fact that

	if s.saveInlineCache {
		cacheExports = append(cacheExports, newInlineCacheOpt())
	}

is only set for the SolveOpt, which is not sent via the ExportRequest, as a result code such as

https://github.com/earthly/buildkit/blob/earthly-main/exporter/containerimage/writer.go#L167 gets set to nil, which then propigates to
https://github.com/earthly/buildkit/blob/f46745e0958c6942be03b9d49684d804e7bdffc4/exporter/containerimage/writer.go#L744-L750 not being called.

@alexcb alexcb added the type:bug Something isn't working label Jan 30, 2023
@alexcb
Copy link
Collaborator Author

alexcb commented Jan 31, 2023

The cacheExporter data is actually sent in -- there shouldn't be a need to update the protos;

I think https://github.com/earthly/buildkit/blob/f46745e0958c6942be03b9d49684d804e7bdffc4/solver/llbsolver/bridge.go#L246-L256 might be the problem.

Before we call exp.Exporter.Export, we need to inrich the inp with cache entries.

I tried some code similar to:

	return inBuilderContext(ctx, b.builder, exp.Exporter.Name(), id, func(ctx context.Context, g session.Group) error {
		sessionIDs := session.AllSessionIDs(g)
		if len(sessionIDs) == 0 {
			return fmt.Errorf("group has no session IDs") // shouldnt happen
		}
		sessionID := sessionIDs[0]

		cacheExporters, inlineCacheExporter := splitCacheExporters(exp.CacheExporters)

		if inlineCacheExporter != nil {
			for k, res := range refs { // TODO Solver code uses cached.Refs (figure out if they are different)
				dtic, err := inlineCache(ctx, inlineCacheExporter.Exporter, res, exp.Exporter.Config().Compression(), g)
				if err != nil {
					fmt.Printf("inlineCache failed %v\n", err)
					return err
				}
				if dtic != nil {
					fmt.Printf("inline cache add metadata2 %s\n", dtic)
					inp.AddMeta(fmt.Sprintf("%s/%s", exptypes.ExporterInlineCache, k), dtic)
				}
			}
		}

		if len(cacheExporters) > 0 {
			bklog.G(ctx).Warnf("got %d cacheExporters (via --wait-block's Export call), but don't know what to do with them", len(cacheExporters))
		}

		var err error
		_, err = exp.Exporter.Export(ctx, inp, sessionID)
		return err
	})

I tried reconstructing the cached struct with

        res := &exporter.Source{}
	for k, ref := range refs {
		res.AddRef(k, ref)
	}
	for k, v := range metadata {
		res.AddMeta(k, v)
	}

but it must be a CachedResult and not a generic Result; so a call must be made to NewCachedResult, which only currently happens in solver/edge.go. Continuing down this path of type-errors and copying and pasting code has become a headwind.

The first Export function made use of a hack to pass the exporter from the Solver to the llbBridge by setting j.SetValue(keyEarthlyExporterInstance, &exp), then reaching back into the job (j). But I have no idea if it would even be possible to propagating the results from the edge.go code into the Solver then into the bridge -- and if it were it would be very hacky.

Ultimately I think we need to re-evaluate how the Export llbBridge code can trigger the Solver.Solve function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
Status: Icebox
Development

No branches or pull requests

1 participant