container-structure-test icon indicating copy to clipboard operation
container-structure-test copied to clipboard

Metadata Test should support all fields from ContainerConfig

Open dzavalkinolx opened this issue 7 years ago • 6 comments

Full list of fields is here: https://github.com/docker/engine-api/blob/master/types/container/config.go#L36

In my case I'm preparing nginx image which should be compatible with openshift. That's why I would like to test `"User": "nginx".

dzavalkinolx avatar Mar 09 '18 22:03 dzavalkinolx

Definitely. Will try and get this in the next release.

nkubala avatar Mar 12 '18 01:03 nkubala

cc @datwiz

nkubala avatar Apr 18 '18 21:04 nkubala

Have started work on this and mapped out which container config fields can be modified during image build vs runtime settings.

Wanting to confirm this feature should only include metadata tests for those fields that are set statically against the image as opposed to dynamically at container runtime. For example, AFAIK, the hostname and domainname fields are only populated on running containers, not images.

For reference, I'm using Docker for Mac 18.03-ce, so there may be some minor differences in different versions or platforms.

Working with the assumption of only including fields set against the image I will be adding the additional following metadata tests:

  • OnBuild
  • Shell+
  • StopSignal
  • User

+ Looks like Shell was added to fsouza/go-dockerclient in the last 2 months but not yet vendored in. (StopTime also impacted) The version currently pegged is from 31-Aug-17 (commit 98edf3e)

Done Build Inst Docker Tar Attribute Config Type cst type
X CMD X X Cmd []string []string
X ENTRYPOINT X X Entrypoint []string []string
X ENV X X Env []string []EnvVar
[ ] ONBUILD X X OnBuild []string []string
[ ] SHELL dep ug X Shell []string []string 
    dep ug nil StopTimeout *int  
  indirect  X X ArgsEscaped bool  
    empty empty AttachStderr bool  
    empty empty AttachStdin bool  
    empty empty AttachStdout bool  
    false false NetworkDisabled bool  
    X X OpenStdin bool  
    X X StdinOnce bool  
    false false Tty bool  
X LABEL X X Labels map[string]string []Label
X EXPOSE X X ExposedPorts map[string]struct{} []string
X VOLUME X X Volumes map[string]struct{} []string
    empty empty Domainname string  
    empty empty Hostname string  
    sha hash sha hash Image string  
    empty empty MacAddress string  
[ ]  STOPSIGNAL X X StopSignal string string
[ ]  USER X X User string string
X WORKDIR X X Workdir string string

datwiz avatar Apr 20 '18 02:04 datwiz

Also pulling in the latest updates to master I noticed that the metadata tests are being skipped, not included in the set of tests in pkg/v2/types/structure.go runAll(). Is that intentional at this point in the refactor?

datwiz avatar Apr 20 '18 06:04 datwiz

@datwiz really nice work here! You're absolutely right, we only want to support config fields that are set statically in the images, not in running containers. We have some work going on in a separate PR (https://github.com/GoogleContainerTools/container-structure-test/pull/82) that will add support for containerRunOpts, and in that case we can look into supporting other config fields in the metadata tests, but what you have is fine for now.

I had fixed the metadata tests not being run in https://github.com/GoogleContainerTools/container-structure-test/commit/a42f38b99381b28affda088cac4bda001757b9e6#diff-51c8b7cc0592214aede72d81164812ab, but we just merged another refactor and it looks like that changed didn't get picked up. I'll fix that now, thanks for pointing that out!

nkubala avatar Apr 20 '18 18:04 nkubala

Working on refactoring the metadata tests into grouped types to make test case additions easier. Checking if there were any issues with standardising the test case failure messages: Image <test> <actval_from_image> does not match expected value: <expval_from_config>

An example:

=== RUN: Metadata Test
--- FAIL
Error: Image envvar "SOME_KEY"="SOME_VAL" does not match expected value: "NOT_SOME_VAL"
Error: Image envvar "NOT_EMPTY_VAR_01" not found
Error: Image label "not-localnet.localdomain.commit_hash" not found
Error: Image label "localnet.my-domain.my-label"="my test label" does not match expected value: "not my label"
Error: Image cmd[1] "-h" does not match expected value: "-x"
Error: Image workdir "/tmp/asdf" does not match expected value: "/not/tmp"
Error: Image user "test-user" does not match expected value: "not-test-user"

I've pushed a WIP view to my local fork metadata.go

datwiz avatar Apr 22 '18 01:04 datwiz