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

feat(dockerimage condition) supports a list of architectures #986

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

dduportal
Copy link
Contributor

Fix #962

This PR introduces a new feature for the conditions of the resource dockerimage.
You can now specify a list of architectures instead of a single one, to simplify the manifests.

Not valid for sources (sources are not multi-valued) and targets (same). @olblak is there a way to tune the JSON schema to avoid it to proposes architectures for sources and targets?

Test

To test this pull request, you can run the following commands:

make build test

Example:

checkTemurinDebianDockerImage:
    kind: dockerimage
    name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
    disablesourceinput: true
    spec:
      architectures:
        - amd64
        - arm64
        - arm/v7
        - s390x
      image: eclipse-temurin
      tag: '{{source "lastVersion" }}-jdk-focal'

returns:

CONDITIONS:
===========

checkTemurinDebianDockerImage
----------------------------------
✔ The Docker image index.docker.io/library/eclipse-temurin:11.0.16.1_1-jdk-focal (amd64) exists and is available.
✔ The Docker image index.docker.io/library/eclipse-temurin:11.0.16.1_1-jdk-focal (arm64) exists and is available.
✔ The Docker image index.docker.io/library/eclipse-temurin:11.0.16.1_1-jdk-focal (arm/v7) exists and is available.
✔ The Docker image index.docker.io/library/eclipse-temurin:11.0.16.1_1-jdk-focal (s390x) exists and is available.

Additional Information

Tradeoff

Writing looooooong manifests?

checkTemurinDebianDockerImageAmd64:
    kind: dockerimage
    name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
    disablesourceinput: true
    spec:
      architecture: amd64
      image: eclipse-temurin
      tag: '{{source "lastVersion" }}-jdk-focal'
checkTemurinDebianDockerImageArm64:
    kind: dockerimage
    name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
    disablesourceinput: true
    spec:
      architecture: arm64
      image: eclipse-temurin
      tag: '{{source "lastVersion" }}-jdk-focal'
  checkTemurinDebianDockerImages390x:
    kind: dockerimage
    name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
    disablesourceinput: true
    spec:
      architecture: s390x
      image: eclipse-temurin
      tag: '{{source "lastVersion" }}-jdk-focal'
  checkTemurinDebianDockerImagesArmv7:
    kind: dockerimage
    name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
    disablesourceinput: true
    spec:
      architecture: arm/v7
      image: eclipse-temurin
      tag: '{{source "lastVersion" }}-jdk-focal'

Potential improvement

  • Deprecate the current parameter architecture in favor of the plural form (requires a validation rule for sources and target though)
  • Support of both OS and architectures

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
@dduportal dduportal added enhancement New feature or request condition Modify condition resource resource-docker Resource of kind Docker Image labels Nov 17, 2022
@olblak
Copy link
Member

olblak commented Nov 17, 2022

Deprecate the current parameter architecture in favor of the plural form (requires a validation rule for sources and target though)

I wouldn't do that since multiple architecture is not supported for sources.
And there is not dockerimage target.

Support of both OS and architectures
I thought you wanted to use "platform". The same approach than docker-compose files

Anyway the current PR is already a great one and complementary with my ongoing effort with docker-compose
I let you merge it if you are happy

@dduportal
Copy link
Contributor Author

Deprecate the current parameter architecture in favor of the plural form (requires a validation rule for sources and target though)

I wouldn't do that since multiple architecture is not supported for sources. And there is not dockerimage target.

👍

@dduportal
Copy link
Contributor Author

Support of both OS and architectures
I thought you wanted to use "platform". The same approach than docker-compose files

Yes I do, but it is another PR (different subject than here).

Anyway the current PR is already a great one and complementary with my ongoing effort with docker-compose I let you merge it if you are happy

❤️

@dduportal dduportal merged commit 343bd79 into updatecli:main Nov 18, 2022
@dduportal dduportal deleted the feat/gh-962 branch November 18, 2022 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
condition Modify condition resource enhancement New feature or request resource-docker Resource of kind Docker Image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Support multiple architectures in conditions for Docker images/digest
2 participants