flux: Readme: Improve documentation #291

kksudo · 2025-06-22T19:10:14Z

Description

Improve the Flux plugin documentation

Fixes #214

Flux: v0.2.0 can't create plugins/flux directory #214

illume · 2025-06-24T16:31:51Z

Thanks! It looks good to me... but the main maintainer of the flux plugin is away this week, so I'll wait till they are back to review/merge.

Would you mind updating the commit messages to follow the commit format we use?

flux: Update the docs to use ephemeral dirs

Signed-off-by: Kirill <[email protected]> (cherry picked from commit d4b75d2) Signed-off-by: Kirill <[email protected]>

Signed-off-by: Kirill <[email protected]>

kksudo · 2025-06-24T18:01:55Z

Would you mind updating the commit messages to follow the commit format we use?

Done

skoeva

LGTM

ashu8912

can you please explain this change, is this an issue?? Why would we have a permission issue here???

kksudo · 2025-07-02T17:14:20Z

The security context is mandatory to copy changes, without it, you will get an error: cp: can't create directory '/build/plugins/flux': Permission denied

Also, there is no statefulset app, that is why use the PV with PVC is not correct, as the alternative I added an option to use an empty dir.

can you please explain this change, is this an issue?? Why would we have a permission issue here???

kksudo · 2025-07-02T17:16:33Z

This PR resolve issue #214
Issue here.

illume · 2025-07-03T17:45:35Z

flux/README.md

          - /bin/sh
          - -c
-          - mkdir -p /build/plugins && cp -r /plugins/* /build/plugins/
+          - mkdir -p /build/plugins && cp -r /plugins/* /build/plugins/ && chown -R 100:101 /build


Oh... I just realised the 100:101 here is the headlamp user and group from the alpine headlamp Dockerfile container?

Can we use chown -R headlamp:headlamp /build here instead?

How about a comment explaining why the chown is necessary?

Oh... I just realised the 100:101 here is the headlamp user and group from the alpine headlamp Dockerfile container?

Yes
https://github.com/headlamp-k8s/plugins/blob/main/Dockerfile#L45
Actually we can improve also this line, to define exactly the number of the group to be more clear.

Can we use chown -R headlamp:headlamp /build here instead?

yes, but it's not a true way, because from image to image the number of the headlamp group could be diff.
It's a better to update the Dockerfile to set up a static number, smth like:

# Create a group with specific GID and user with specific UID RUN addgroup -g 101 -S headlamp && \ adduser -u 100 -S -G headlamp headlamp

I can update it, WDYT ?

Thanks. I understand I think... so if someone uses headlamp:headlamp in say their own custom Debian image maybe it's different from an alpine image?

Yeah, I think it's a good change to make.

illume · 2025-07-03T17:46:08Z

flux/README.md

        image: ghcr.io/headlamp-k8s/headlamp-plugin-flux:latest
        imagePullPolicy: Always
        name: headlamp-plugins
+        securityContext:


This part here is required because of the chown used above? Or is it for something else?

Could you please add a comment to there explaining why it's necessary?

The securityContext overwrite the USER headlamp non root user.
Without it, we don't have enough permissions to do the dirs with non headlamp owner.

Ok, that makes sense now. Thanks.

Could you please add a comment about that in there for future readers?

Ok, that makes sense now. Thanks.

Could you please add a comment about that in there for future readers?

I can, it's not a big deal.
But, are you sure if it's necessary? Because it's a pretty simple setting. Moreover, we can't annotate every line. We should do it, only if it's some exception or TODO or workaround.

WDYT?

Umm i don't understand this change we already did chwon above so we would not want to run this as root right??

maybe this is what we need here

securityContext: runAsNonRoot: true runAsUser: 100 runAsGroup: 101 privileged: false

Yes, it's true. What is the user who run this cmd?
Without it, you will fail.
Btw, you can try to run without these changes to check it.

Umm i don't understand this change we already did chwon above so we would not want to run this as root right??

maybe this is what we need here

securityContext: runAsNonRoot: true runAsUser: 100 runAsGroup: 101 privileged: false

@ashu8912
Again, did you check the root cause of these changes here ?

illume · 2025-07-03T17:50:52Z

@ashu8912 @kksudo I added a few notes/questions at points where I think we can add comments to make this a bit easier to understand. Please check them to see what you think?

illume · 2025-07-03T17:54:28Z

Added "Fixes #214" to PR description.

kksudo · 2025-07-03T18:08:51Z

@ashu8912 @kksudo I added a few notes/questions at points where I think we can add comments to make this a bit easier to understand. Please check them to see what you think?

Answered

ashu8912 · 2025-07-07T18:09:51Z

flux/README.md

        image: ghcr.io/headlamp-k8s/headlamp-plugin-flux:latest
        imagePullPolicy: Always
        name: headlamp-plugins
+        securityContext:


Umm i don't understand this change we already did chwon above so we would not want to run this as root right??

maybe this is what we need here

securityContext: runAsNonRoot: true runAsUser: 100 runAsGroup: 101 privileged: false

ashu8912

I think i finally understand the changes here.

kksudo force-pushed the feat/improve_readme branch from 8c367f2 to 6956588 Compare June 22, 2025 20:11

illume changed the title ~~Improve documentation~~ flux: Readme: Improve documentation Jun 23, 2025

illume added the flux flux plugin related issues label Jun 23, 2025

illume requested a review from ashu8912 June 24, 2025 16:32

kksudo force-pushed the feat/improve_readme branch from 6956588 to 22ba9d1 Compare June 24, 2025 18:00

kksudo added 2 commits June 24, 2025 21:01

flux: docs: Update the docs, resolve permissions issue

0e407ac

Signed-off-by: Kirill <[email protected]> (cherry picked from commit d4b75d2) Signed-off-by: Kirill <[email protected]>

flux: docs: Update the docs to use ephemeral dirs

f617c23

Signed-off-by: Kirill <[email protected]>

kksudo force-pushed the feat/improve_readme branch from 22ba9d1 to f617c23 Compare June 24, 2025 18:01

kksudo mentioned this pull request Jun 25, 2025

Fix multiarch build #290

Closed

skoeva approved these changes Jun 26, 2025

View reviewed changes

ashu8912 reviewed Jul 2, 2025

View reviewed changes

illume reviewed Jul 3, 2025

View reviewed changes

illume requested a review from ashu8912 July 7, 2025 11:14

ashu8912 requested changes Jul 7, 2025

View reviewed changes

ashu8912 approved these changes Jul 28, 2025

View reviewed changes

ashu8912 merged commit a236a5c into headlamp-k8s:main Jul 28, 2025
3 checks passed

flux: Readme: Improve documentation #291

flux: Readme: Improve documentation #291

Uh oh!

Conversation

kksudo commented Jun 22, 2025 • edited by illume Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

illume commented Jun 24, 2025

Uh oh!

kksudo commented Jun 24, 2025

Uh oh!

skoeva left a comment

Choose a reason for hiding this comment

Uh oh!

ashu8912 left a comment

Choose a reason for hiding this comment

Uh oh!

kksudo commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kksudo commented Jul 2, 2025

Uh oh!

illume Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illume Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illume Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kksudo Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illume commented Jul 3, 2025

Uh oh!

illume commented Jul 3, 2025

Uh oh!

kksudo commented Jul 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashu8912 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kksudo commented Jun 22, 2025 •

edited by illume

Loading

kksudo commented Jul 2, 2025 •

edited

Loading

illume Jul 3, 2025 •

edited

Loading

illume Jul 7, 2025 •

edited

Loading

illume Jul 3, 2025 •

edited

Loading

kksudo Jul 7, 2025 •

edited

Loading