search

ansible-community / images

Ansible container image definitions meant for ansible-test and Execution Environments
GNU General Public License v3.0
14 stars 12 forks source link

Adds 2.15 ee image and build update #61

Closed anweshadas closed 1 year ago

anweshadas commented 1 year ago

Updates the Github Action for the new execution environement file

Spredzy commented 1 year ago

Few comments here:

  1. Hard pinning on core should be removed. The latest available version should be pulled.
  2. I don't think this GHA is appropriate for this action. This one is triggered on push, PR and on schedule. For now and until we can eventually linked it to a new tag available on core automatically, this should be run on-demand, with either the version specified in a form or even better introspected from the job to create the proper tag number to push to the registry with
softwarefactory-project-zuul[bot] commented 1 year ago

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/5725acca357d472a8b4ae89712d48ccb

:heavy_check_mark: community-images-ansible-test-build SUCCESS in 20m 37s

felixfontein commented 1 year ago

The EE should also be listed in the EE readme: https://github.com/ansible-community/images/blob/main/execution-environments/README.md#available-images

felixfontein commented 1 year ago

I don't think this GHA is appropriate for this action. This one is triggered on push, PR and on schedule.

It is appropriate. The GHA action makes sure that the EEs can actually be built. This needs to be checked regularly (cron), when PRs are created (they could modify the EE), and it's a good idea to also check that after merging a PR.

For now and until we can eventually linked it to a new tag available on core automatically, this should be run on-demand, with either the version specified in a form or even better introspected from the job to create the proper tag number to push to the registry with

EEs should be built a lot more often than on releases, simply to make sure that updates to the underlying base image (like security updates) are incorporated frequently.

One big question is whether there should be multiple active tags for one EE at the same time, or not ("latest" only). If there are multiple tags, rebuilding them frequently requires a lot of resources.

softwarefactory-project-zuul[bot] commented 1 year ago

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/e15bf9fb15b8483f950e1be716dadd8c

:heavy_check_mark: community-images-ansible-test-build SUCCESS in 22m 38s

softwarefactory-project-zuul[bot] commented 1 year ago

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/2c2246a679ac4786a8836dc7ae081e7c

:heavy_check_mark: community-images-ansible-test-build SUCCESS in 24m 19s

softwarefactory-project-zuul[bot] commented 1 year ago

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/2b9d3bfb343045c7b602a74059a001f3

:heavy_check_mark: community-images-ansible-test-build SUCCESS in 15m 58s

gotmax23 commented 1 year ago

merged commit 31c0265 into main

https://github.com/ansible-community/images/pull/61#discussion_r1296362165 was never addressed.

Andersson007 commented 1 year ago

merged commit 31c0265 into main

#61 (comment) was never addressed.

@gotmax23 thanks! @felixfontein apologies! @anweshadas could you please take a look?

felixfontein commented 1 year ago

This PR doesn't look ready for merging to me.

The names don't match (see my comment), only one of the two EEs was added to CI, and the tests are over-specific (they test things that are not specified in the EE config file, like the exact Fedora version; and other things like the ansible version aren't checked at a ll).

Also for 2.15-with-ansible8 the collection versions are pinned, and there is no indication of how / when these will be updated. If we pin versions, we need a procedure to keep them up to date. (I would argue we should always use the latest non-prerelease.)

felixfontein commented 1 year ago

(Also the README was not updated: https://github.com/ansible-community/images/blob/main/execution-environments/README.md)

felixfontein commented 1 year ago

Also according to https://forum.ansible.com/t/community-execution-environment-image-content-and-location/262/14 the names should be community-ee-minimal and community-ee-base, and should not include 2.15 (since that will be wrong from 2023-11-06 on anyway, as the images do not specify a specific ansible-core version).

gotmax23 commented 1 year ago

It looks like the images were already published last month without this PR being finalized, so it seems this process was done backwards.

AlanCoding commented 1 year ago

I'm glad to see this go in because it upgrades to the builder v3 format, and recommending anything built with the old base images is a bad idea. I think I agree with the other comments about naming, as it's hard to follow what the version numbers even refer to.

Andersson007 commented 1 year ago

It looks like the images were already published last month without this PR being finalized, so it seems this process was done backwards.

@gotmax23 yes, you're right, once we agreed in the forum topic about the content, we published those images to proceed with the Getting started EE guidelines. It was a blocker.

felixfontein commented 1 year ago

It has been two weeks since this PR was merged, but the problems mentioned here (that should have prevented a merge) haven't been fixed. @anweshadas @Andersson007

Andersson007 commented 1 year ago

@felixfontein thanks for reminding! We remember about this and once @anweshadas 's back, she'll take a look

felixfontein commented 1 year ago

I created a follow-up PR to fix some of the issues in #65.