siderolabs / talos-vmtoolsd

VMware tools implementation for the Talos Kubernetes platform, using govmomi and Talos' apid
Apache License 2.0
28 stars 14 forks source link

chore: enable kres #17

Closed frezbo closed 7 months ago

frezbo commented 8 months ago

Enable kres and update repo branch settings.

Notable changes:

Pull requests require status checks and approval.

Usually a PR can be merged by adding a /m comment. We do this in the org repos to avoid merge commits that has no trusted GPG signature (GitHub merge commits doesn;t do a ff merge and adds their own GPG signature)

I have disabled the GPG signature to be enforced for a commit.

Fixes: #14

JinxCappa commented 8 months ago

To all parties involved,

I would like to make a strong case to have this tool added to the official extensions repo. While standard Talos users can continue to use this tool the original way designed, having this be an extension simplifies implementation as extensions can be baked into the image so that now all vSphere VMs can enable this right from clone/deploy. The simplest and easiest way to do that I believe is using Image Factory.

The biggest reason honestly to add this as an official extension is for Omni I think. Being able to create an ova from within Omni with the vmtoolsd extension already added would be great. An even bigger reason is for upgradability. While now Omni connectivity settings can be added to custom Talos builds with custom extensions, not unifying these two repos breaks Omni's design of managing and upgrading these VMs as Omni does not allow for incorporation of non official extensions into images built from within it's interface. At least I believe so; who knows, could be wrong.

So again, I hope my reasons sound well.. reasonable. I actually do have a commit burning a hole in my fork of the extensions repo begging for a pull request but I digress and will wait for the maintainers of this repo to come to a decision.

robinelfrink commented 8 months ago

Hi @JinxCappa,

I agree having talos-vmtoolsd in the official extensions repository would be great. I don't see any reason why that couldn't be done, even if this repository builds it's own extension-container.

Note however that this repository is a community effort, and none of the maintainers are affiliated with Sidero. They just offered to host the repository under `siderolabs/'.

So my advice would be to just create that PR, parallel to what we are doing here.

JinxCappa commented 8 months ago

I appreciate the response and am looking forward to submitting my PR.

As a question to @frezbo, as this repo stands now the releases are not tagged with 'v{semver}' but just the plain version instead. With the addition of kres to this repo, will this be changed to match the release format of siderolabs repos? I ask as this will affect pulling the source to build the extension as well as the renovate regex comment in the PR. View the below links for reference:

pkgs.yaml vars.yaml

If so, I don't have an issue submitting my PR to extensions but note that it will have to be modified if this PR gets finalized and reformats release tags.

frezbo commented 8 months ago

I appreciate the response and am looking forward to submitting my PR.

As a question to @frezbo, as this repo stands now the releases are not tagged with 'v{semver}' but just the plain version instead. With the addition of kres to this repo, will this be changed to match the release format of siderolabs repos? I ask as this will affect pulling the source to build the extension as well as the renovate regex comment in the PR. View the below links for reference:

pkgs.yaml vars.yaml

If so, I don't have an issue submitting my PR to extensions but note that it will have to be modified if this PR gets finalized and reformats release tags.

Yes, it would starting with v, at least the CI expects that standard for tags

robinelfrink commented 7 months ago

@frezbo thanks! Container generated using the modified Makefile works as DaemonSet as well as Talos System Extension.

Judging by the new github workflow code, we would still need to manually create tags, right?

frezbo commented 7 months ago

Judging by the new github workflow code, we would still need to manually create tags, right?

yes, just make sure it has the v prefix

frezbo commented 7 months ago

@frezbo thanks! Container generated using the modified Makefile works as DaemonSet as well as Talos System Extension.

Judging by the new github workflow code, we would still need to manually create tags, right?

if you could +1 (approval), I can then get it merged

frezbo commented 7 months ago

/m