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

buildah-config: created and created-by confusion #595

Closed
taqtiqa-mark opened this issue Apr 17, 2018 · 40 comments
Closed

buildah-config: created and created-by confusion #595

taqtiqa-mark opened this issue Apr 17, 2018 · 40 comments

Comments

@taqtiqa-mark
Copy link

The OCI Spec v1.0.1 suggests that neither --created nor --created-by ought to be user configurable.

  • Created:

    combined date and time at which the image was created, formatted as defined by RFC 3339, section 5.6.

  • Created by:

    The command which created the layer.

@taqtiqa-mark taqtiqa-mark changed the title buildah-config: created vs created_by confusion buildah-config: created vs created-by confusion Apr 17, 2018
@taqtiqa-mark taqtiqa-mark changed the title buildah-config: created vs created-by confusion buildah-config: created and created-by confusion Apr 17, 2018
@rhatdan
Copy link
Member

rhatdan commented Apr 17, 2018

@TomSweeneyRedHat Can you remove this option, but add the option when commit is executed.

@TomSweeneyRedHat
Copy link
Member

@rhatdan, I'm 99% sure what you're asking for. I think you want me to remove the created and created-by options from buildah config and then determine those values when the config happens and save it to the container. I'll make that so, holler if I need to adjust.

@taqtiqa-mark
Copy link
Author

Believe this would also close #537

@rhatdan
Copy link
Member

rhatdan commented Apr 17, 2018

@TomSweeneyRedHat Yes on the first statement, but created and createdby should be populated on buildah commit, not no buildah config.

@pixdrift
Copy link
Contributor

pixdrift commented Apr 17, 2018

@taqtiqa-mark #537 isn't just the format the date is displayed in, the primary issue is that local timezone isn't being applied when the time is displayed to the user.

The spec isn't clear on which timezone the container 'created' should be in, I would assume UTC. This should probably be raised back as an issue to opencontainers.

@TomSweeneyRedHat
Copy link
Member

@pixdrift the created date shown in buildah inspect is in UTC. However when we display the creation date of the image via buildah images we convert the time displayed by that command to the time of the machine that runs the command as determined by its locale. We don't change the time inside the image itself though.

@pixdrift
Copy link
Contributor

pixdrift commented Apr 17, 2018

@TomSweeneyRedHat this is how I would expect it to work, but my ticket #537 shows that it is reporting the raw UTC time off the container and not adjusting for my local time (AEST which is UTC+10). In my situation, it looks like all container operations happened 10 hours in the past.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 17, 2018

@pixdrift @TomSweeneyRedHat I think if the spec format was followed then the timezone decoration would explain the 10 hour difference.

My view is to keep things a simple as possible ... so just stick with UTC throughout and format the time as the spec states. Chopping and changing is problematic.
Also I think a buildah use case is that it will be used in scripted scenarios where you can't guarantee that data output by buildah and passed around is being passed between two machines in the same TZ or even with the same locale settings.

To clarify the use case. An artifact is created on two CI services. Which one finished first?

Reporting or recording with TZ or locale adjustments just opens up a world of pain for scripted scenarios.
With respect to humans reading the output data rather than scripts consuming the output data... I think we can reasonably assume that the container creator user base is sophisticated enough not to be thrown off too much by time stamps formatted as the spec indicates?

@pixdrift
Copy link
Contributor

pixdrift commented Apr 17, 2018

@taqtiqa-mark I am not suggesting that the date/time be anything other than UTC in the container. I was raising that the spec doesn't stipulate that UTC should be used for this date/time and the spec probably should define this categorically.

My issue in #537 is that the displayed CREATED AT time in buildah images is showing the raw UTC time from the container, and not displaying in local time in the user interface. There is no suggestion that anything should be changed in the container configuration at all, just that what is displayed to the user should be timezone adjusted.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 17, 2018

@pixdrift ack. I'm arguing displayed data should be UTC too. Humans are only one type of consumer of buildah output. Scripts are another important group. It's much easier for a Human to recognize a UTC time than to write scripts and pipelines that coordinate/adjust time zone/locale changes or differences.

Automate everything is very nearly possible, and these sorts of "trivial" issues can cause havoc, then result in elaborate, fragile, expensive to maintain code bases.

@pixdrift
Copy link
Contributor

pixdrift commented Apr 17, 2018

@taqtiqa-mark ack. I'd argue that buildah images primary consumer is humans. If there is a requirement to interface with the container attributes programatically you could use output from buildah inspect through jq. I accept others may argue the counter.

buildah inspect 9000983b8701 | jq '.OCIv1.created'
"2018-04-05T03:40:01.984312955Z"

-edit-
Remove reference to Docker, not really relevant.

@TomSweeneyRedHat
Copy link
Member

I'll let @rhatdan chime in on what we should be displaying time wise. But just to be totally clear, the times that we store for the containers and images are UTC times. What we display is UTC time with the exception of the time displayed in the buildah images command which converts the time to locale time based on #537.

@pixdrift I'm a little confused (it's late, so likely me) by your reply above: "but my ticket #537 shows that it is reporting the raw UTC time off the container and not adjusting for my local time (AEST which is UTC+10)." #537 was for the image, not the container. Is the latest Buildah (from GitHub, don't think we've spun a rpm with the fix yet) not showing that for you? Is there a spot that the time for a container is being displayed that should be showing UTC in your opinion.

I believe #544 addresses #537, OK to close #53 now?

@pixdrift
Copy link
Contributor

pixdrift commented Apr 17, 2018

@TomSweeneyRedHat, my issue #537 is about display only, and is in buildah images, you're absolutely correct. I am commenting on it in this thread because it was suggested above that this issue would also close #537, but in my view they are unrelated.

I will follow up #537 in that issue and provide feedback, apologies I hadn't seen your fix.

@pixdrift
Copy link
Contributor

@TomSweeneyRedHat, @taqtiqa-mark To summarise above, this proposal/suggestion looks good, my comments were in response to how it related to #537, which is now closed. Crisis over 😄

@rhatdan
Copy link
Member

rhatdan commented Apr 18, 2018

How about buildah images does display in LocaleTime and buildah images --json returns in UTC.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 18, 2018

Appreciate the proposal.
The std Linux approach is to pipe command output via cut etc. - keep simple things simple.
I've tried parsing json through bash etc. I don't think it is a solution here.
Ideally you shouldn't need more than what is in busy-box to consume buildah output.

May be a --pipe or --pipe-output option that produces delimited (un-percent-encoded output). Trouble here is selecting a delimiter that is not part of the content defined in the spec.... may be | as a default? Is <tab> part of the OCI spec content?

Anyway, this could be made robust by providing an option --pipe-delimiter <char>?
That way if someone encounters output with |, etc. they can workaround it.

Agree all dates and times sent in response to --pipe should be UTC.

@rhatdan
Copy link
Member

rhatdan commented Apr 18, 2018

How about if we check if the output is a tty, if yes, then it is converted, if not then it get UTC. Although this might confuse certain users. Perhaps we should add a flag to tell it to not convert.

buildah --utc
buildah --noformat
?

@pixdrift
Copy link
Contributor

pixdrift commented Apr 18, 2018

@taqtiqa-mark, ls displays in local time but you can use tools like find to determine date ranges without parsing the date in the console ouput of ls. Is the issue that the output is from inspect is json? or that the date isn't utc? Using local time to display output isn't breaking a unix convention.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 18, 2018

@rhatdan, I tend to prefer explicit --pipe --utc over implicit --noformat which presumes the user knows what format the raw is. Even more explicit and self descriptive/documenting is --pipe --utc-3339. If (when?) the date-time spec changes, at least there won't be confusion about one flag in legacy code ;)

Detecting a TTY rather than following a --pipe flag: I'd go for the explicit flag it seems simpler. I don't have direct experience with linux vs solaris vs *BSD vs... to know if reliably detecting TTY across all of them is simple. I have enough experience to know nothing is a simple as it seemed at the outset ;)

@pixdrift, json issue and not utc issue yes, json overly complex. I understand by using json the developers get some freedom, the tradeoff is that now buildah is one more tool's output for which busy-box isn't sufficient - note not all cases will buildah --pipe be parsed immediately. Stored in a file and passed along to be parsed (cat pipe-file.txt|...) is another another regular use case.

Thanks for your time, thought and effort.

@pixdrift
Copy link
Contributor

@taqtiqa-mark, can you provide some examples of the buildah commands with proposed switches with proposed/expected output?

eg.
buildah images --pipe
buildah images --pipe --utc

@pixdrift
Copy link
Contributor

pixdrift commented Apr 18, 2018

I was interested in how this situation was handled in Docker, and it looks like the solution in Docker is to use the more generic --format parameter that is built in to present json output in a format that is easily parseable.
https://docs.docker.com/config/formatting/

When looking for --format in the other projectatomic utilities, I found this in the skopeo README.md.

NOT TODO
provide a format flag - just use the awesome jq

Should there be some discussion about how all the tools under projectatomic umbrella (ie. buildah, podman, skopeo) provide parseable output so that it is consistent?

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 19, 2018

[update] change from buildah images to buildah inspect per @pixdrift 's comment below.

Refering to the configuration list in issue #591 table

Ideal workflow would look like this, unless I missed something buildah push... doesn't return the container ID in a way that helps....

OCI_ID=$(buildah push ...)
OCI_CONFIG="$(buildah inspect --pipe --pipe-delimiter='|' |grep ${OCI_ID})"
IFS='|' read arch authors cmd created created_by description  remain1 <<< ${OCI_CONFIG}
IFS='|' read documentation entrypoint env licenses oci_id os port ref_name remain2 <<< ${remain1}
IFS='|' read revision source title url user vendor remain3 <<< ${remain2}
IFS='|' read version volume workingdir <<< ${remain3}

The assumptions are:

  1. the output is in alphabetical order of the buildah-config flags.
  2. All flags have a default value if they are not set - even if it is empty strings or some placeholder.
  3. Date and time are output as UTC-3339

Hope that helps?

P.S @pixdrift to my mind this isn't so much a 'format' as making the buildah command line interface tool support the linux piping convention (philosophy?). For example a, say, buildah library or web interface would like chose a different interface convention/spec.
The OCI spec does impose a format on date and time, but even here we don't have to even advertise that - we know that the ${created} extracted from buildah is 'guaranteed' to be comparable.

@pixdrift
Copy link
Contributor

pixdrift commented Apr 19, 2018

@taqtiqa-mark, what do you expect the output of buildah images --pipe --pipe-delimiter='|' to be? Can you give a console output example of this, without processing.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 19, 2018

[update] Added the OCI ID hash in this example as the first field which we grep on - could also place it in alphabetical order, wherever 'oci_id' then is placed. If you were worried about collision you could output the full hash. It's not for human parsing so no reason why not be safer than sorry.

OCI_CONFIG='4272a169ec1c|amd_64|"Daniel Walsh, pixdrift"|"/bin/sh consul-entrypoint.sh"|"me oh <my@mailinator.com>"|http://www.google.com|1|@|3|&|'
IFS='|' read oci_id arch authors cmd description documentation remain1 <<< ${OCI_CONFIG}
echo $arch
echo $authors 
echo $cmd 
echo $description 
echo $documentation 
echo $remain1

Is that what you wanted?

@pixdrift
Copy link
Contributor

pixdrift commented Apr 19, 2018

@taqtiqa-mark I am interested in what the output of this command would be. No parsing or interpretation, just what you would like to see printed to the console?
buildah images --pipe --pipe-delimiter='|'

My view is that the Unix Philosophy is absolutely supported in buildah's current form. You are referring to elements that you want to extract from the spec. The method provided to interrogate the spec of the container is buildah inspect. The output of buildah inspect is json, a format that is easily parsed by jq, which is a single program that does a single thing very well (parses json). jq is available on all Linux platforms i've worked on.

I don't fully understand the constraint of limiting to commands available in busybox, can you provide some details on the kind of environment you're building containers in that only has busybox?

-edit-

I should point out I have no say on the outcome, I am just a user of the tools. I am genuinely interested in how you're using it.

@pixdrift
Copy link
Contributor

pixdrift commented Apr 19, 2018

If I can make another point for jq. The jq solution is more robust than using positional parameters. Even if the output items are sorted alphabetically, if anything is added to the spec (even elements you don't utilise) the script will break. With buildah inspect + jq, you are referencing specific items by name that won't be impacted by additional elements/fields being added to the spec or command output.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 19, 2018

[update] Removed wall of text to the salient point.

what the output of this command would be

See OCI_CONFIG above. This is hypothetical output. echo $OCI_CONFIG in the above example and that will show you the output. It won't change from what OCI_CONFIG has been set to.

The method provided to interrogate the spec of the container is buildah inspect

Fine, just change buildah images .... in the above to buildah inspect .... I was just following on from some ones comment about buildah images ... I'm not fussed.

I'm not disputing jq's beauty or anything about it.

I really don't see the value or ease of json piping being so overwhelming that std conventional plain string piping should be dropped.

@rhatdan
Copy link
Member

rhatdan commented Apr 19, 2018

I would think the format command would be best to satisfy your needs then you can format the output of buildah images any way you want.

Take a look at podman images --format would similar functionality from buildah satisfy your needs?

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 19, 2018

@rhatdan and @pixdrift I've given the concrete use case.
In 6 lines I show an image build and how buildah-config settings can be easily parsed into env variables using prexisting tools. There is no busybox requirement, once you've got the buildahfile all you need is read and a shell.

(For later readers - the buildah-push and buildah-inspect behavior above is vapoware at the time of writing)

I'm not familiar with jq or podman. Perhaps you can show how you envision the same results can be achieved using those approaches and then the choice will be clearer?

@pixdrift
Copy link
Contributor

pixdrift commented Apr 19, 2018

@taqtiqa-mark, I have provided the --format documentation for Docker in an above post, which describes its operation. What @rhatdan is proposing is a re-implementation of this capability in buildah as this capability has already been replicated in podman. To summarise, it essentially includes a json parsing/templating mechanism in the tool itself to remove the need for jq.

@pixdrift
Copy link
Contributor

@taqtiqa-mark, As requested, example using jq in current config (random fields selected)

buildah inspect centos:7 | jq '.OCIv1.architecture+"|"+.OCIv1.os+"|"+.OCIv1.config.Cmd[]'
"amd64|linux|/bin/bash"

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 21, 2018

Thanks @pixdrift for taking the time to do that. I appreciate we are all doing this in what would otherwise be family or recreation time.

A couple of observations.

  1. It'll be useful to show the full PoC you have in mind that would set the same suite of environment variables I gave in my use case. Ack buildah doesn't (yet?) support setting all that configuration data through buildah config.
  2. You say the idea is to make the functionality/behavior possible available without jq. Can you share how you think/hope/intend that will work/look - the example uses jq
  3. It'll be useful if all a user needs to know to retrieve, say os, is that it was set via buildah config --os=.... In the proposal to use json (even without jq) it seems I've now got to know in detail buildah's implementation of the spec. Ideally I should not even have to know any spec detail. When either the spec or the implementation of it changes, as they surely will, my scripts should not break so long as os is somehow still available in the same N series of buildah.

@pixdrift
Copy link
Contributor

pixdrift commented Apr 22, 2018

@taqtiqa-mark

  1. I don't know what @rhatdan / @TomSweeneyRedHat have planned for setting this config, I was only commenting on the display/extraction of the information once it's set. I will let them comment on that.

  2. The suggestion from @rhatdan is to use the same --format style parameter that is used in docker. The docker capability is described here:
    https://docs.docker.com/config/formatting/

Examples of displaying specific information formatted from inspect is as follows:
docker inspect --format '{{json .Mounts}}' container

So that would become
buildah inspect --format '{{json .Mounts}}' container

This example can be extended to output any combination of fields/elements from the inspect result in whichever format you need. ie. you could use a --format string to provide the piped output you are suggesting, or alternatively display it as key=value. This solution doesn't utilise jq and provides the formatting in the tool itself, so would achieve your goal of buildah + busybox being able to parse the details.

I can put together a more complete example from docker and post it.

  1. Agree, you will need to use buildah inspect to determine where in the json the field is written and then reference that with --format. Perhaps a feature request could be to make buildah config have a read switch, or read the value if no value is provided? (like a set / get) and that could be used as an interface to get the detail.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Apr 22, 2018

@pixdrift : Thanks. Striving to keep things simple and avoid the need to consult docs etc. Not easy to write intuitive software, I know. Esp when user base is so diverse.

The request for the complete equivalent example is to confirm my hunch: it's so painful, no one wants to be the one to write it out ;) And that's without facing the prospect of dealing with syntax errors, capitalization errors, . decoration, etc. etc. etc. etc.

@rhatdan
Copy link
Member

rhatdan commented Apr 23, 2018

@pixdrift @taqtiqa-mark Lets add the format and see if we can satisfy your needs with that. Should be simple to back port the changes from podman to buildah images. @umohnani8 Could you look at that?

@pixdrift
Copy link
Contributor

@rhatdan agreed, format to align with other tools would be a good solution.

@rhatdan
Copy link
Member

rhatdan commented Apr 24, 2018

I am not sure what we are missing now?

buildah inspect --format '{{.OCIv1.Created}}' fedora-working-container 
2018-03-07 20:51:34.488688562 +0000 UTC

If you want it in namevalue pairs

buildah inspect --format 'created="{{.OCIv1.Created}}"' fedora-working-container 
created="2018-03-07 20:51:34.488688562 +0000 UTC"

@pixdrift
Copy link
Contributor

pixdrift commented Apr 24, 2018

@rhatdan, looks fine to me. @taqtiqa-mark, does that meet your requirement/expectation?

@umohnani8
Copy link
Member

@rhatdan add --format to buildah inspect only or to images as well?

@rhatdan
Copy link
Member

rhatdan commented Apr 24, 2018

@umohnani8 We have it now I believe.

@rhatdan rhatdan closed this as completed May 25, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants