⚠️ Follow XDG Directory standard for config/data/... files #6913

cwrau · 2022-07-13T13:41:46Z

What this PR does / why we need it: This PR switches the hardcoded paths ($HOME/.cluster-api) to be XDG conform ($XDG_CONFIG_HOME/cluster-api).

This removes clutter in the users' home directory.

Open questions:

I tried adding backwards compatibility to the old path inside the reader_viper.go but I'm not sure if this works for everything
The script cmd/clusterctl/hack/create-local-repository.py creates a dev-repository at $HOME/.cluster-api/dev-repository. I see three solutions to this;
- The script uses an xdg library, which has to be installed on the users' system, which we'll mention in the docs
- The script does everything by itself, without needing dependencies
  - This could be a lot of work, especially for Windows and macOS, as I can't test these systems
- The documentation will mention to overwrite XDG_CONFIG_HOME while using the script, this could be weird
I personally vote for the first possibility

linux-foundation-easycla · 2022-07-13T13:41:50Z

The committers listed above are authorized under a signed CLA.

✅ login: cwrau / name: Chris Werner Rau (a0d0c1d)

k8s-ci-robot · 2022-07-13T13:41:54Z

Welcome @cwrau!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-07-13T13:41:54Z

Hi @cwrau. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

apricote · 2022-07-13T13:46:28Z

/ok-to-test

cwrau · 2022-07-14T07:52:43Z

The apidiff is failing, but that is expected, what can I do?

killianmuldoon · 2022-07-14T09:21:13Z

The api diff isn't merge blocking if the maintainers think this is worth merging without an API revision - the check is really informing rather than blocking for API changes so don't worry about trying to pass it :)

killianmuldoon · 2022-07-14T09:22:01Z

The api diff isn't merge blocking if the maintainers think this is worth merging without an API revision - the check is really informing rather than blocking for API changes so don't worry about trying to pass it - it would be good if you could sign the CLA though 🙂

cwrau · 2022-07-14T09:27:03Z

The api diff isn't merge blocking if the maintainers think this is worth merging without an API revision - the check is really informing rather than blocking for API changes so don't worry about trying to pass it - it would be good if you could sign the CLA though slightly_smiling_face

Yes, my boss is working on the CLA 👍️

cwrau · 2022-07-27T07:17:03Z

CLA has been accepted 👍️

/help

killianmuldoon · 2022-07-27T08:44:34Z

Good to hear! I'm not sure what you're looking for with the help tag though 😆

cwrau · 2022-07-27T10:33:25Z

Good to hear! I'm not sure what you're looking for with the help tag though 😆

I just wanted to add the help-wanted label, but apparently that doesn't work this way 🤣

killianmuldoon · 2022-07-27T10:40:24Z

I think we normally only add that label to issues where we want someone to take them up - are you looking to have someone else pick up this PR? If so it might be better to create an issue describing the goals, link this draft PR and then add the help wanted tag.

cwrau · 2022-07-27T10:46:34Z

Ah, ok, then I misunderstood the point of that label 😅

No, I can keep this PR 👍

sbueringer · 2022-08-16T12:20:48Z

@cwrau Is the PR ready for review? (just asking because we usually use Draft/WIP to ~ signal implementation is still in progress and folks should wait for reviews except if they are explicitly asked to provide a review)

cc @ykakarap

cwrau · 2022-08-16T16:58:56Z

@sbueringer

Ah, I see, yeah, I still need to have the questions answered 👍

By anyone, really 😅

sbueringer · 2022-08-16T17:31:15Z

Got it.

cc @ykakarap
I'll also take a closer look soon

ykakarap

Ah, I see, yeah, I still need to have the questions answered 👍

By anyone, really 😅

I will take a look at this.

ykakarap

Looks like according to XDG documentation if XDG_CONFIG_HOME is not defined it should default to $HOME/.config. Meaning, we will effectively move from $HOME/.cluster-api to $HOME/.config/cluster-api in the general case.

I do not have strong opinions on doing this move.

Regarding your first question - I think establishing backward compatibility with $HOME/.cluster-api should be possible. This would also mean unless we break the contract both the XDG_CONFIG_HOME and $HOME/.cluster-api will need to be supported.

Regarding your second question - Are you suggesting that we move the dev-repository location also to XDG_CONFIG_HOME? If so, It might be easier to just build the support into the script we have. Looks like we dont need all of xdg library to begin with and can just have the logic that reads the XDG_CONFIG_HOME env var and default to the appropriate value if it is not defined. This would be simpler and would reduce external dependencies. As you pointed out I am not sure if this will be complicated for other OS environments. We will need more information before we can decide.

The documentation will mention to overwrite XDG_CONFIG_HOME while using the script, this could be weird

I am not sure why a user will have to override XDG_CONFIG_HOME. Can you please explain?

dims · 2022-08-19T23:58:18Z

prior art from kubectl https://github.com/kubernetes/kubernetes/pulls?q=is%3Apr+XDG

richardcase · 2023-06-08T08:07:10Z

Looks like with a rebase this is good to go.

go.mod

sbueringer · 2023-06-13T08:35:41Z

Only did a high-level review given all the lgtm's above. Looks good to me, just one nit.

Thank you very much for the patience, would be great to get this into the next release.

…. files feat(clusterctl): use XDG_CONFIG_HOME for script as well

richardcase · 2023-06-13T08:42:52Z

/lgtm

k8s-ci-robot · 2023-06-13T08:42:58Z

LGTM label has been added.

Git tree hash: 0a1f3bb26081b57b62b2140d39d471cff41080f7

sbueringer · 2023-06-13T09:08:47Z

Thx! lgtm (from a high-level review)

killianmuldoon

/lgtm
/approve

Thanks again for following through on this!

k8s-ci-robot · 2023-06-13T09:33:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: killianmuldoon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [killianmuldoon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cwrau · 2023-06-13T14:34:18Z

🥳🎉

this is following changes to upstream to make it follow the xdg configuration directory. see kubernetes-sigs/cluster-api#6913 for more information.

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 13, 2022

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 13, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 13, 2022

k8s-ci-robot requested review from fabriziopandini and killianmuldoon July 13, 2022 13:41

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 13, 2022

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 13, 2022

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 19, 2022

ykakarap reviewed Aug 18, 2022

View reviewed changes

ykakarap reviewed Aug 19, 2022

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 29, 2022

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2023

cwrau force-pushed the feature/follow-xdg-standard branch from 7c7737d to 101fdf3 Compare April 24, 2023 08:46

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 24, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 18, 2023

cwrau force-pushed the feature/follow-xdg-standard branch from 101fdf3 to e69ace0 Compare May 22, 2023 07:55

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 7, 2023

cwrau force-pushed the feature/follow-xdg-standard branch from e69ace0 to 9c75855 Compare June 12, 2023 13:47

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 12, 2023

sbueringer reviewed Jun 13, 2023

View reviewed changes

go.mod Outdated Show resolved Hide resolved

feature(clusterctl): Follow XDG Directory standard for config/data/..…

a0d0c1d

…. files feat(clusterctl): use XDG_CONFIG_HOME for script as well

cwrau force-pushed the feature/follow-xdg-standard branch from 9c75855 to a0d0c1d Compare June 13, 2023 08:41

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 13, 2023

killianmuldoon reviewed Jun 13, 2023

View reviewed changes

k8s-ci-robot assigned killianmuldoon Jun 13, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2023

k8s-ci-robot merged commit 69d4494 into kubernetes-sigs:main Jun 13, 2023
20 checks passed

k8s-ci-robot added this to the v1.5 milestone Jun 13, 2023

killianmuldoon mentioned this pull request Jun 13, 2023

📖 Small doc fixes for XDG_CONFIG_HOME #8843

Merged

This was referenced Aug 11, 2023

⚠️ Bump CAPI to v1.5.0 metal3-io/cluster-api-provider-metal3#1079

Merged

Bump CAPI to v1.5.0 metal3-io/metal3-dev-env#1268

Merged

ryjogo mentioned this pull request Feb 20, 2024

Overrides not working in .cluster-api/ only in .config/cluster-api #10179

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚠️ Follow XDG Directory standard for config/data/... files #6913

⚠️ Follow XDG Directory standard for config/data/... files #6913

cwrau commented Jul 13, 2022

linux-foundation-easycla bot commented Jul 13, 2022 •

edited

k8s-ci-robot commented Jul 13, 2022

k8s-ci-robot commented Jul 13, 2022

apricote commented Jul 13, 2022

cwrau commented Jul 14, 2022

killianmuldoon commented Jul 14, 2022

killianmuldoon commented Jul 14, 2022

cwrau commented Jul 14, 2022 •

edited

cwrau commented Jul 27, 2022 •

edited

killianmuldoon commented Jul 27, 2022

cwrau commented Jul 27, 2022

killianmuldoon commented Jul 27, 2022

cwrau commented Jul 27, 2022

sbueringer commented Aug 16, 2022 •

edited

cwrau commented Aug 16, 2022 •

edited

sbueringer commented Aug 16, 2022

ykakarap left a comment

ykakarap left a comment

dims commented Aug 19, 2022

richardcase commented Jun 8, 2023

sbueringer commented Jun 13, 2023

richardcase commented Jun 13, 2023

k8s-ci-robot commented Jun 13, 2023

sbueringer commented Jun 13, 2023

killianmuldoon left a comment

k8s-ci-robot commented Jun 13, 2023

cwrau commented Jun 13, 2023

⚠️ Follow XDG Directory standard for config/data/... files #6913

⚠️ Follow XDG Directory standard for config/data/... files #6913

Conversation

cwrau commented Jul 13, 2022

linux-foundation-easycla bot commented Jul 13, 2022 • edited

k8s-ci-robot commented Jul 13, 2022

k8s-ci-robot commented Jul 13, 2022

apricote commented Jul 13, 2022

cwrau commented Jul 14, 2022

killianmuldoon commented Jul 14, 2022

killianmuldoon commented Jul 14, 2022

cwrau commented Jul 14, 2022 • edited

cwrau commented Jul 27, 2022 • edited

killianmuldoon commented Jul 27, 2022

cwrau commented Jul 27, 2022

killianmuldoon commented Jul 27, 2022

cwrau commented Jul 27, 2022

sbueringer commented Aug 16, 2022 • edited

cwrau commented Aug 16, 2022 • edited

sbueringer commented Aug 16, 2022

ykakarap left a comment

Choose a reason for hiding this comment

ykakarap left a comment

Choose a reason for hiding this comment

dims commented Aug 19, 2022

richardcase commented Jun 8, 2023

sbueringer commented Jun 13, 2023

richardcase commented Jun 13, 2023

k8s-ci-robot commented Jun 13, 2023

sbueringer commented Jun 13, 2023

killianmuldoon left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 13, 2023

cwrau commented Jun 13, 2023

linux-foundation-easycla bot commented Jul 13, 2022 •

edited

cwrau commented Jul 14, 2022 •

edited

cwrau commented Jul 27, 2022 •

edited

sbueringer commented Aug 16, 2022 •

edited

cwrau commented Aug 16, 2022 •

edited