project-stacker / stacker

Build OCI images natively from a declarative format
https://stackerbuild.io
Apache License 2.0
208 stars 34 forks source link

Bug: the 'import' fallback fails for 'prerequisites' #592

Closed hallyn closed 8 months ago

hallyn commented 9 months ago

(This bug was found by @ariel-miculas and @andaaron was the one who guessed at the 'import' being the problem)

stacker version

v1.0.0-rc11

Describe the bug

'import:' in stacker.yaml has been replaced by 'imports:'. Those files end up under /stacker/imports/ instead of directly under /stacker. In order to provide some backward compatible support as users switch over, there is fallback support for 'import:' which is scheduled to be removed in the future.

This support however breaks mysteriously when using the 'prerequisites' feature.

To reproduce

  1. Configuration
cat >> stacker-req.yaml << EOF
base:
    from:
      type: docker
      url: docker://busybox:latest
    build_only: true
    import:
        - helloworld
    run: |
      cp /stacker/helloworld /

EOF
cat >> stacker.yaml << EOF
config:
  prerequisites:
      - stacker-req.yaml

buildenv:
    from:
        type: built
        tag: base
    import:
        - stacker://base/helloworld
    run: |
        cp /stacker/helloworld /h3

EOF
stacker build
  1. Client tool used The latest stacker pre-release.
  2. Seen error
    serge@serge-l-PF3DENS3 ~/delme/stacker-imports$ ./stacker build
    'import' directive used in layer 'buildenv' inside file 'stacker.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
    'import' directive used in layer 'base' inside file '/home/serge/delme/stacker-imports/stacker-req.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
    'import' directive used in layer 'base' inside file '/home/serge/delme/stacker-imports/stacker-req.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
    preparing image base...
    copying /home/serge/delme/stacker-imports/helloworld
    loading docker://busybox:latest
    Copying blob 9ad63333ebc9 done  
    Copying config 3f57d9401f done  
    Writing manifest to image destination
    Storing signatures
    + cp /stacker/helloworld /
    'import' directive used in layer 'buildenv' inside file '/home/serge/delme/stacker-imports/stacker.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
    preparing image buildenv...
    + cp /stacker/helloworld /h3
    cache miss because layer definition was changed
    error: couldn't find a cache of base layer for buildenv: base
    error: exit status 1

Expected behavior

When the two stackerfiles are combined into one, the build succeeds:

serge@serge-l-PF3DENS3 ~/delme/stacker-imports$ cat stacker-import-allinone.yaml 
base:
    from:
      type: docker
      url: docker://busybox:latest
    build_only: true
    import:
        - helloworld
    run: |
      cp /stacker/helloworld /

buildenv:
    from:
        type: built
        tag: base
    import:
        - stacker://base/helloworld
    run: |
        cp /stacker/helloworld /h3

serge@serge-l-PF3DENS3 ~/delme/stacker-imports$ ./stacker build -f !$
./stacker build -f stacker-import-allinone.yaml
'import' directive used in layer 'base' inside file 'stacker-import-allinone.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
'import' directive used in layer 'buildenv' inside file 'stacker-import-allinone.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
'import' directive used in layer 'base' inside file '/home/serge/delme/stacker-imports/stacker-import-allinone.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
'import' directive used in layer 'buildenv' inside file '/home/serge/delme/stacker-imports/stacker-import-allinone.yaml' is deprecated. Support for 'import' will be removed in releases after 2025-01-01. Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. See https://github.com/project-stacker/stacker/issues/571 for migration.
preparing image base...
using cached copy of /home/serge/delme/stacker-imports/helloworld
loading docker://busybox:latest
Copying blob 9ad63333ebc9 skipped: already exists  
Copying config 3f57d9401f done  
Writing manifest to image destination
Storing signatures
cache miss because layer definition was changed
+ cp /stacker/helloworld /
preparing image buildenv...
failed to send signal urgent I/O condition no such process
cache miss because layer definition was changed
+ cp /stacker/helloworld /h3
filesystem buildenv built successfully

Screenshots

No response

Additional context

No response

mikemccracken commented 8 months ago

I recently hit this in the context of stacker publish failing to publish after a successful build, and running with --debug points to the flag wasLegacyImport being different in the cache vs what the publish command saw:

It looks like by the time the cache is updated from the build, the correct value of the wasLegacyImport flag has been lost. The stacker.yaml in question here did have the old import syntax, and this instance of the bug goes away if we just fix the syntax in the stacker.yaml.

[2024-03-20T01:23:45.628Z] cached: types.Layer{From:types.ImageSource{Type:"built", Url:"", Tag:"minbase", Insecure:false}, ...snip ... Annotations:map[string]string(nil), OS:(*string)(0xc000ab7320), Arch:(*string)(0xc000ab7330), Bom:(*types.Bom)(nil),
 wasLegacyImport:false}

[2024-03-20T01:23:45.631Z] new: types.Layer{From:types.ImageSource{Type:"built", Url:"", Tag:"minbase", Insecure:false}, ...snip ... Annotations:map[string]string(nil), OS:(*string)(0xc000591e60), Arch:(*string)(0xc000591e70), Bom:(*types.Bom)(nil), 
wasLegacyImport:true}

[2024-03-20T01:23:45.632Z] cache miss because layer definition was changed