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

docker/install: Fix latest image install on lima #480

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Oct 30, 2024

latest is not a valid git tag or revision to get the matching systemd unit files.
Look up the exact source git commit from the org.opencontainers.image.revision image config label.

@crazy-max
Copy link
Member

crazy-max commented Oct 30, 2024

https://github.com/docker/actions-toolkit/actions/runs/11591165912/job/32270352253?pr=480#step:10:745

        wget https://raw.githubusercontent.com/moby/moby/27.3.1/contrib/init/systemd/docker.service         -O /etc/systemd/system/docker.service
        wget https://raw.githubusercontent.com/moby/moby/27.3.1/contrib/init/systemd/docker.socket         -O /etc/systemd/system/docker.socket

leads to 404: https://raw.githubusercontent.com/moby/moby/27.3.1/contrib/init/systemd/docker.service

Looks like commit sha is not resolved and /blob is missing? Guess it should be: https://github.com/moby/moby/blob/41ca978a0a5400cc24b274137efa9f25517fcc0b/contrib/init/systemd/docker.service

@vvoland vvoland force-pushed the docker-install-image-latest-fix branch from 946f9ec to 5898099 Compare October 30, 2024 10:37
@vvoland
Copy link
Contributor Author

vvoland commented Oct 30, 2024

Oh right, that URL only worked for tags, not refs.

EDIT: The url is correct, but for some reason the gitCommit isn't set. Looking into it.

Comment on lines 145 to 146
const manifest = await moby.getPlatformManifest(tag);
const config = await moby.getConfig<ImageConfig>(manifest.config.digest);
Copy link
Member

@crazy-max crazy-max Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think labels and annotations will only be on the manifest list (index) config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at docker buildx imagetools inspect moby/moby-bin:latest --raw , index doesn't have these:

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:c52ff4ad684b729f597eefacf1abc65913dcb3423071b71997cd271690c8a9b7",
      "size": 1630,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:fda20c7387897ee54811fd74b96472c4a454a62f29b5eb595caaeb4e04f01bd5",
      "size": 567,
      "annotations": {
        "vnd.docker.reference.digest": "sha256:c52ff4ad684b729f597eefacf1abc65913dcb3423071b71997cd271690c8a9b7",
        "vnd.docker.reference.type": "attestation-manifest"
      },
      "platform": {
        "architecture": "unknown",
        "os": "unknown"
      }
...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image config has them though:

docker buildx imagetools inspect  moby/moby-bin:latest@sha256:fe77a2f844e3db621de180b88e0e8c5439dd6802e4e6efcc5371be4a05e252d3 --raw | jq
{
  "architecture": "amd64",
  "config": {
    "Env": [
      "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
    ],
    "WorkingDir": "/",
    "Labels": {
      "org.opencontainers.image.created": "2024-09-20T18:10:20.768Z",
      "org.opencontainers.image.description": "The Moby Project - a collaborative project for the container ecosystem to assemble container-based systems",
      "org.opencontainers.image.licenses": "Apache-2.0",
      "org.opencontainers.image.revision": "41ca978a0a5400cc24b274137efa9f25517fcc0b",
      "org.opencontainers.image.source": "https://github.com/moby/moby",
      "org.opencontainers.image.title": "moby",
      "org.opencontainers.image.url": "https://github.com/moby/moby",
      "org.opencontainers.image.version": "27.3.1"
    }
  },

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@vvoland vvoland force-pushed the docker-install-image-latest-fix branch from 5898099 to 82cd3fd Compare October 30, 2024 10:49
@vvoland vvoland marked this pull request as draft October 30, 2024 10:50
@vvoland
Copy link
Contributor Author

vvoland commented Oct 30, 2024

Drafted for now and added some debug logs. Will undraft once fixed.

@vvoland vvoland force-pushed the docker-install-image-latest-fix branch 14 times, most recently from 8c34034 to c1a54eb Compare October 30, 2024 13:54
@vvoland
Copy link
Contributor Author

vvoland commented Oct 30, 2024

Fixed! The issues were:

  • Trying to get the config for the darwin platform (there's no Engine build for darwin)
  • Casting the application/vnd.oci.image.config.v1+json object directly to a ImageConfig struct (when it's actually Image that has the config embedded as config 🤦🏻)

@vvoland vvoland marked this pull request as ready for review October 30, 2024 14:37
@vvoland vvoland requested a review from crazy-max October 30, 2024 14:37
Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

src/docker/install.ts Outdated Show resolved Hide resolved
`latest` is not a valid git tag or revision to get the matching systemd
unit files.
Look up the exact source git commit from the
`'org.opencontainers.image.revision` image config label.

Signed-off-by: Paweł Gronowski <[email protected]>
@vvoland vvoland force-pushed the docker-install-image-latest-fix branch from c1a54eb to 61c10b2 Compare October 30, 2024 14:45
@crazy-max crazy-max merged commit 853d5fa into docker:main Oct 30, 2024
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants