Skip to content

feat: Allow use of akmods module#71

Merged
gmpinder merged 13 commits into
mainfrom
42-feat-akmods-module
Feb 22, 2024
Merged

feat: Allow use of akmods module#71
gmpinder merged 13 commits into
mainfrom
42-feat-akmods-module

Conversation

@gmpinder
Copy link
Copy Markdown
Member

@gmpinder gmpinder commented Feb 18, 2024

The akmods module require having the /rpms directory put into /tmp/rpms. By default we will mount the akmods image with the main-{{ os_version }} tag.

If a user supplies base for the akmods module in their recipe, it will pull that image tag instead and mount the resulting /rpms.

modules:
- type: akmods
  base: surface
  install:
  - openrazer

This would pull the image ghcr.io/ublue-os/akmods:surface-39.

A user can also supply nvidia-version with the numerical version of the driver you would like to use. Doing so will mount the appropriate akmods-nvidia image with the version of the driver you want in the tag.

modules:
- type: akmods
  nvidia-version: 545
  install:
  - nvidia

This would pull the image ghcr.io/ublue-os/akmods-nvidia:main-39-545 and ghcr.io/ublue-os/akmods:main-39.

This uses bind mount like all the other modules so these files will not persist into the final image.

The akmods module require having the /rpms directory put into /tmp/rpms.
By default we will mount the akmods image with the main-latest tag. If a user
supplies version-tag for the akmods module in their recipe, it will pull that
image version instead and mount the resulting /rpms. This uses bind mount like all the
other modules so these files will not persist into the final image
@gmpinder gmpinder self-assigned this Feb 18, 2024
@gmpinder gmpinder linked an issue Feb 18, 2024 that may be closed by this pull request
HexSleeves
HexSleeves previously approved these changes Feb 18, 2024
Copy link
Copy Markdown
Contributor

@HexSleeves HexSleeves left a comment

Choose a reason for hiding this comment

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

We can probs my start using macro templates that make this a bit more human readable

@gmpinder
Copy link
Copy Markdown
Member Author

We can probs my start using macro templates that make this a bit more human readable

Yeah that will be done for this issue

@gmpinder
Copy link
Copy Markdown
Member Author

This latest commit will fail, will come back to this later

@gmpinder gmpinder enabled auto-merge (squash) February 19, 2024 05:37
Comment thread integration-tests/test-repo/config/recipe.yml
@gmpinder
Copy link
Copy Markdown
Member Author

Ok this most recent change now puts all the akmods rpms into a stage. I also added logic to pull both the nvidia and non-nvidia images so that users don't have to specify 2 separate modules to install nvidia and base rpms.

Comment thread templates/Containerfile Outdated
@fiftydinar
Copy link
Copy Markdown
Contributor

fiftydinar commented Feb 21, 2024

There is no need to use 2 recipe steps, as nvidia-version of akmods is only used to install Nvidia drivers & nothing else.

  - type: akmods
    base: surface
    nvidia-version: 545
    install:
      - nvidia
      - openrazer
  - type: akmods
    install:
      - openrgb

I think it's better to have just this, where with nvidia-version yaml entry, you don't need to specify -nvidia in install section at all.

  - type: akmods
    base: surface
    nvidia-version: 545
    install:
      - openrgb
      - openrazer      

I can update bash module to suit this by parsing nvidia-version yaml entry. I would adapt Universal Blue's Nvidia script.
If nvidia-version entry doesn't exist or is empty, it won't install Nvidia driver..

So basically, this is how it should be running in order:

  1. Main, Surface or Asus akmods, which installs regular akmods (surface-39 in this case)
  2. Nvidia akmods, which only install Nvidia driver (following the variant & version of akmods: surface-39-545 in this case)

@xynydev
Copy link
Copy Markdown
Member

xynydev commented Feb 21, 2024

Wait, so does definining an Nvidia version automatically pull in Nvidia drivers? I was always under the impression that, since there's Nvidia and non-Nvidia versions of all akmods, there's a subtle difference there and only akmods for Nvidia images can be installed on Nvidia images. I don't get why anyone would reinstall Nvidia, when -surface-nvidia has Nvidia.

@fiftydinar
Copy link
Copy Markdown
Contributor

fiftydinar commented Feb 21, 2024

Wait, so does definining an Nvidia version automatically pull in Nvidia drivers? I was always under the impression that, since there's Nvidia and non-Nvidia versions of all akmods, there's a subtle difference there and only akmods for Nvidia images can be installed on Nvidia images. I don't get why anyone would reinstall Nvidia, when -surface-nvidia has Nvidia.

I tried to dive into those nvidia akmod images, to see what is in them, but It reports that image doesn't exist for some reason. But I assume that it's only used to install Nvidia akmod & nothing else based on Bazzite Containerfile. If it contains Nvidia akmod only, then I believe there shouldn't be any overwrite happening.

So yeah, normal akmods work on -nvidia images too, as Nvidia akmod is compiled for Fedora kernel (+ Asus & Surface if specified).

If someone is using Universal Blue image & wants Nvidia, they should use -nvidia tagged image & install akmods regularly.

But if someone will use vanilla Fedora image, he would need to install Nvidia akmod as specified in the recipe with nvidia-version. I believe we mentioned we wanted to support vanilla Fedora images too, right?

I think that some assistance from Universal Blue folks would be nice here.

@xynydev
Copy link
Copy Markdown
Member

xynydev commented Feb 21, 2024

Thanks for the partial clarification.

I believe we mentioned we wanted to support vanilla Fedora images too, right?

This module currently doesn't officially support upstream Fedora, but if it's determined to be usable I guess that could be extended.

@fiftydinar
Copy link
Copy Markdown
Contributor

fiftydinar commented Feb 21, 2024

I made a PR in bling for akmods bash module to adapt with this PR vision. My PR is above this comment.

I would like to hear thoughts on my PR, on if everything is synced well between bash & cli module.

@gmpinder
Copy link
Copy Markdown
Member Author

I made a PR in bling for akmods bash module to adapt with this PR vision. My PR is above this comment.

I would like to hear thoughts on my PR, on if everything is synced well between bash & cli module.

Ok so in this case I would only be concerned about pulling the proper base image?

@gmpinder gmpinder disabled auto-merge February 21, 2024 22:20
@fiftydinar
Copy link
Copy Markdown
Contributor

fiftydinar commented Feb 21, 2024

I made a PR in bling for akmods bash module to adapt with this PR vision. My PR is above this comment.
I would like to hear thoughts on my PR, on if everything is synced well between bash & cli module.

Ok so in this case I would only be concerned about pulling the proper base image?

Yeah.

Recipe format is this one I proposed above:

  - type: akmods
    base: surface
    nvidia-version: 545
    install:
      - openrgb
      - openrazer      

@gmpinder
Copy link
Copy Markdown
Member Author

I made a PR in bling for akmods bash module to adapt with this PR vision. My PR is above this comment.
I would like to hear thoughts on my PR, on if everything is synced well between bash & cli module.

Ok so in this case I would only be concerned about pulling the proper base image?

Yeah.

Recipe format is this one I proposed above:

  - type: akmods
    base: surface
    nvidia-version: 545
    install:
      - openrgb
      - openrazer      

So then it would only download the ghcr.io/ublue-os/akmods:surface-39? Will those nvidia RPMs also be available in the same image? When I download ghcr.io/ublue-os/akmods-nvidia:surface-39-545 it only had the rpm for nvidia kmod in /rpms.

@fiftydinar
Copy link
Copy Markdown
Contributor

fiftydinar commented Feb 21, 2024

So then it would only download the ghcr.io/ublue-os/akmods:surface-39? Will those nvidia RPMs also be available in the same image? When I download ghcr.io/ublue-os/akmods-nvidia:surface-39-545 it only had the rpm for nvidia kmod in /rpms.

With this format, it downloads both akmods:surface-39 & akmods-nvidia:surface-39-545 image. Main image does not contain Nvidia akmods, hence the need for 2 images.

I set it up in bash module to install regular akmods 1st & Nvidia drivers last.
I also set a check that if driver is other then 470 or 545, it fails.
If Nvidia drivers are detected in the system, it won't install Nvidia drivers.
Although, unused Nvidia akmod image would remain in this scenario, due to user error.
Not a big concern, since Nvidia akmod image is small.

When only base is specified as surface, it downloads akmods:surface-39 normally & installs akmods normally.

@gmpinder
Copy link
Copy Markdown
Member Author

gmpinder commented Feb 21, 2024

So then it would only download the ghcr.io/ublue-os/akmods:surface-39? Will those nvidia RPMs also be available in the same image? When I download ghcr.io/ublue-os/akmods-nvidia:surface-39-545 it only had the rpm for nvidia kmod in /rpms.

With this format, it downloads both akmods:surface-39 & akmods:surface-39-545 image.

I set it up in bash module to install regular akmods 1st & Nvidia drivers last. I also set a check that if driver is other then 470 or 545, it fails. If Nvidia drivers are detected in the system, it won't install Nvidia drivers. Although, unused Nvidia akmod image would remain in this scenario, due to user error. Not a big concern, since Nvidia akmod image is small.

When only base is specified as surface, it downloads akmods:surface-39 normally & installs akmods normally.

.... so I do keep my current implementation? I'm confused.

Given the module set in my test:

modules:
  - type: akmods
    base: surface
    nvidia-version: 545
    install:
      - nvidia
      - openrazer
  - type: akmods
    install:
      - openrgb

It does the following:

  • Reads every akmods module
  • Creates a stage name based on the values (or lack thereof) of base and nvidia-version
    • If base is empty, use main
    • If nvidia-version is empty use empty string
  • Copies the rpms from base image (either akmods:main-{version} or akmods:{base}-{version}) /rpms into the stage
  • If nvidia-version was set, copies rpms from nvidia image (akmods-nvidia:{base/main}-{version}-{nvidia-version} into same stage

This produces the following stages:

FROM scratch as stage-akmods-surface-545
COPY --from=ghcr.io/ublue-os/akmods:surface-39 /rpms /rpms
COPY --from=ghcr.io/ublue-os/akmods-nvidia:surface-39-545 /rpms /rpms

FROM scratch as stage-akmods-main
COPY --from=ghcr.io/ublue-os/akmods:main-39 /rpms /rpms

For the RUN instruction, if the type is akmods, it creates a bind mount on the corresponding stage:

RUN \
  # ...
  --mount=type=bind,from=stage-akmods-surface-545,src=/rpms,dst=/tmp/rpms,rw \
  # ...
  && source /tmp/exports.sh && /tmp/modules/akmods/akmods.sh '{"type":"akmods","base":"surface","nvidia-version":545,"install":["nvidia","openrazer"]}'

RUN \
  # ...
  --mount=type=bind,from=stage-akmods-main,src=/rpms,dst=/tmp/rpms,rw \
  # ...
  && source /tmp/exports.sh && /tmp/modules/akmods/akmods.sh '{"type":"akmods","install":["openrgb"]}'

This ensures that there aren't any rpms being overwritten except that which is in the akmods-nvidia images (which from my investigation is only the nvidia rpms) for any given akmods module the user defines.

This currently works as the integration tests are run on the recipe in the pipeline.

@fiftydinar
Copy link
Copy Markdown
Contributor

fiftydinar commented Feb 21, 2024

.... so I do keep my current implementation? I'm confused.

Given the module set in my test:

modules:
  - type: akmods
    base: surface
    nvidia-version: 545
    install:
      - nvidia
      - openrazer
  - type: akmods
    install:
      - openrgb

It does the following:

* Reads every akmods module

* Creates a stage name based on the values (or lack thereof) of `base` and `nvidia-version`
  
  * If `base` is empty, use `main`
  * If `nvidia-version` is empty use empty string

* Copies the rpms from base image (either `akmods:main-{version}` or `akmods:{base}-{version}`) `/rpms` into the stage

* If `nvidia-version` was set, copies rpms from nvidia image (`akmods-nvidia:{base/main}-{version}-{nvidia-version}` into same stage

This produces the following stages:

FROM scratch as stage-akmods-surface-545
COPY --from=ghcr.io/ublue-os/akmods:surface-39 /rpms /rpms
COPY --from=ghcr.io/ublue-os/akmods-nvidia:surface-39-545 /rpms /rpms

FROM scratch as stage-akmods-main
COPY --from=ghcr.io/ublue-os/akmods:main-39 /rpms /rpms

For the RUN instruction, if the type is akmods, it creates a bind mount on the corresponding stage:

RUN \
  # ...
  --mount=type=bind,from=stage-akmods-surface-545,src=/rpms,dst=/tmp/rpms,rw \
  # ...
  && source /tmp/exports.sh && /tmp/modules/akmods/akmods.sh '{"type":"akmods","base":"surface","nvidia-version":545,"install":["nvidia","openrazer"]}'

RUN \
  # ...
  --mount=type=bind,from=stage-akmods-main,src=/rpms,dst=/tmp/rpms,rw \
  # ...
  && source /tmp/exports.sh && /tmp/modules/akmods/akmods.sh '{"type":"akmods","install":["openrgb"]}'

This ensures that there aren't any rpms being overwritten except that which is in the akmods-nvidia images (which from my investigation is only the nvidia rpms) for any given akmods module the user defines.

Your current implementation is good, this is what Is wanted.

The only thing is recipe file format, which has 2 steps, instead of 1, like I proposed above.
I think that 1 step format is much cleaner.

Can you maintain the same logic you mentioned in 1 step akmod format?

@gmpinder
Copy link
Copy Markdown
Member Author

Can you maintain the same logic you mentioned in 1 step akmod format?

Oh absolutely, I just put it in there more as being able to test edge cases like having those multiple stages. So I'm going to keep it as is. This file isn't meant for documentation, only testing.

@gmpinder gmpinder enabled auto-merge (squash) February 21, 2024 22:52
@gmpinder gmpinder merged commit 8931a22 into main Feb 22, 2024
@gmpinder gmpinder deleted the 42-feat-akmods-module branch February 22, 2024 18:41
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.

feat: akmods module

4 participants