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

bootc e2e workflow - build temp image shim with ssh keys around bootc #428

Merged
merged 5 commits into from
May 3, 2024

Conversation

Gregory-Pereira
Copy link
Collaborator

@Gregory-Pereira Gregory-Pereira commented May 2, 2024

The previous PRs have setup our ENV and given us access to the bootc installed instance, now we want to run the e2e tests

@Gregory-Pereira Gregory-Pereira force-pushed the instructlab-testing-framework-4 branch 4 times, most recently from 03db31e to c9b6fa5 Compare May 2, 2024 04:47
@Gregory-Pereira Gregory-Pereira changed the title test I still have access to the box bootc e2e workflow - run the e2e tests May 2, 2024
@lmilbaum lmilbaum force-pushed the instructlab-testing-framework-4 branch 9 times, most recently from 360f280 to be72cb1 Compare May 2, 2024 18:45
@Gregory-Pereira Gregory-Pereira force-pushed the instructlab-testing-framework-4 branch 9 times, most recently from e3fe0d3 to 32f2826 Compare May 2, 2024 21:46
@Gregory-Pereira Gregory-Pereira force-pushed the instructlab-testing-framework-4 branch from 32f2826 to 1685042 Compare May 2, 2024 21:50
training/provision/playbook.yml Outdated Show resolved Hide resolved
@Gregory-Pereira Gregory-Pereira force-pushed the instructlab-testing-framework-4 branch 3 times, most recently from 7642e9a to 6802b0e Compare May 3, 2024 02:00
@Gregory-Pereira Gregory-Pereira force-pushed the instructlab-testing-framework-4 branch 10 times, most recently from 6843b72 to 72a276d Compare May 3, 2024 19:57
@lmilbaum lmilbaum force-pushed the instructlab-testing-framework-4 branch from 72a276d to ee6e914 Compare May 3, 2024 20:10
Signed-off-by: greg pereira <grpereir@redhat.com>
Signed-off-by: greg pereira <grpereir@redhat.com>
Signed-off-by: greg pereira <grpereir@redhat.com>
Signed-off-by: greg pereira <grpereir@redhat.com>
@lmilbaum lmilbaum force-pushed the instructlab-testing-framework-4 branch 3 times, most recently from 548b83b to 5382b76 Compare May 3, 2024 21:17
containers.podman.podman_image:
name: "quay.io/ai-lab/{{ image_name }}:latest"
build:
target: localhost/derivied_image:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo "derived"

Copy link
Contributor

Choose a reason for hiding this comment

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

But um...actually looking at the ansible docs for this module I see
https://docs.ansible.com/ansible/latest/collections/containers/podman/podman_image_module.html#parameter-build/target
"Specify the target build stage to build." which doesn't sound like what's desired here...

I think what you want is to use what you have in target for name actually.

Listen so...I like Ansible, I think it's a powerful swiss army knife that absolutely has its uses.

But this specific code, it doesn't seem like the ansible abstractions are buying us anything over plain old basic shell script executing podman build + podman run...

@@ -37,7 +72,9 @@
privileged: yes
pid_mode: host
command: "bootc install to-filesystem --karg=console=ttyS0,115200n8 --replace=alongside /target"
become: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe this is necessary as there's a toplevel become: true on the whole playbook.

- name: Required packages
ansible.builtin.dnf:
name:
- https://s3.eu-west-2.amazonaws.com/amazon-ssm-eu-west-2/latest/linux_amd64/amazon-ssm-agent.rpm
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why...this beast has deep hooks into everything, it's Amazon wanting to own systems management.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I use it for connecting to the instance for debugging purposes. It will be removed as soon as the workflow starts working

- name: Derived Image Containerfile
ansible.builtin.template:
src: ./templates/Containerfile.j2
dest: /tmp/Containerfile
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit but, predictable filenames in /tmp are a CWE: https://capec.mitre.org/data/definitions/149.html

There's a tempfile module.

Not a big deal, but just noting.

@@ -0,0 +1,9 @@
FROM quay.io/ai-lab/{{ image_name }}:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

podman build already has --from to template this without writing a new containerfile btw.

RUN mkdir /usr/etc-system && \
chown -R root:root /usr/etc-system && \
echo 'AuthorizedKeysFile /usr/etc-system/root.keys' >> /etc/ssh/sshd_config.d/30-auth-system.conf && \
echo {{ ssh_public_key }} > /usr/etc-system/root.keys && \
Copy link
Contributor

Choose a reason for hiding this comment

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

And this one I think is better as a build argument instead of jinja templating, which can be passed already via --build-arg.

@lmilbaum lmilbaum force-pushed the instructlab-testing-framework-4 branch from 5382b76 to cc795e5 Compare May 3, 2024 21:41
Signed-off-by: greg pereira <grpereir@redhat.com>
@lmilbaum lmilbaum force-pushed the instructlab-testing-framework-4 branch from cc795e5 to bcf522f Compare May 3, 2024 22:02
username: "{{ registry_user }}"
password: "{{ registry_password }}"
registry: quay.io
authfile: /etc/containers/auth.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we really want containers/image#1746 ...will see if I or someone else can get back to it.

@Gregory-Pereira Gregory-Pereira marked this pull request as ready for review May 3, 2024 22:11
@Gregory-Pereira Gregory-Pereira changed the title bootc e2e workflow - run the e2e tests bootc e2e workflow - build temp image shim with ssh keys around bootc May 3, 2024
@Gregory-Pereira Gregory-Pereira merged commit e9d5aa8 into main May 3, 2024
1 of 2 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.

3 participants