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

Add a skip to create a gcp repo #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kedaarsridhar20
Copy link

A SKIP to add a new repo called gcp in the SpinKube organization

A SKIP to add a new repo called `gcp` in the SpinKube organization

Signed-off-by: Kedaar Sridhar <[email protected]>

Inspired by the successful implementation of [SKIP 002](https://github.com/spinkube/skips/tree/main/proposals/002-skips) for Azure, we propose to create a new repository under the SpinKube organization for Google Cloud called `gcp` to host any Google Cloud-specific configurations and integrations.

The `gcp` repository will contain the basic Helm chart for deploying SpinKube on GKE clusters. Additionally, it will include a marketplace-specific Helm chart for a future Google Cloud marketplace listing. The owners/maintainers of this repository will be Nick Eberts and Gari Singh from Google, along with Fermyon engineering support for this initial integration.
Copy link

Choose a reason for hiding this comment

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

This is one area I'd like to revisit. The spinkube/azure repo did set the precedent of creating a generic SpinKube chart (basically, an 'umbrella' chart with all the necessary chart deps in place, i.e. spin-operator, cert-manager and kwasm-operator). This same setup could/would work just fine on GCP, right? If so, it'd be great to avoid following the pattern or replicating the same generic chart in every spinkube cloud-provider repo. Perhaps one solution is to break out the generic spinkube chart into a standalone repo with CI/CD to publish somewhere public (eg ghcr.io)?

However, I don't mean to block on this, since my concern isn't specific to GCP. The first version of this repo could follow the same pattern as spinkube/azure and we can always consolidate/re-org at a later date.

Copy link
Author

Choose a reason for hiding this comment

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

The same setup should work for GCP. I always assumed that the later goal would be to consolidate all the top-level charts into a singular "charts" repo, as you mentioned. I think for now, the repos should be separate since there will be marketplace-specific integrations, with an eventual re-org for any top-level charts. The Google PMs are keen to get moving as well, and might have their own steps in mind for executing this integration.

Copy link

Choose a reason for hiding this comment

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

I think for now, the repos should be separate since there will be marketplace-specific integrations

Yes, definitely agree repos should be separate per the marketplace-specific integrations. Thanks, should have clarified that.

Copy link
Contributor

@endocrimes endocrimes Jul 26, 2024

Choose a reason for hiding this comment

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

did set the precedent of creating a generic SpinKube chart (basically, an 'umbrella' chart with all the necessary chart deps in place, i.e. spin-operator, cert-manager and kwasm-operator).

I think I'd continue to avoid this until we're sure of longer term direction. Partially because chart dependencies caused a lot of pain during early testing of the operator, and so letting a couple of "fewer keystroke" options evolve independently before then reconciling seems ok to me. Premature consolidation mostly reduces experimentation + evolution here.

Choose a reason for hiding this comment

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

along with Fermyon engineering support for this initial integration

This should be "SpinKube maintainers" IMO.

@calebschoepp calebschoepp requested review from calebschoepp, flavio, voigt, 0xE282B0 and a team and removed request for flavio, voigt and 0xE282B0 July 24, 2024 15:24
Copy link

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Although I'm not a member of any of the assigned teams for review, I thought I'd inject a bit of momentum by submitting an approval, having been active in the spinkube/azure repo that this is modeled after. Thanks @kedaarsridhar20!

Copy link

@radu-matei radu-matei left a comment

Choose a reason for hiding this comment

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

LGTM, with the one comment about a subset SpinKube maintainers being owners/maintainers of the project.

Copy link
Contributor

@endocrimes endocrimes left a comment

Choose a reason for hiding this comment

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

(with Radu's comment applying here too)

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.

4 participants