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

refactor: make management controller able to create project namespaces again #1482

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

krancour
Copy link
Member

@krancour krancour commented Feb 16, 2024

Since #1431 we rely on a webhook to create Project namespaces synchronously. Although this was a valid decision made for well-documented reasons, one undesired side-effect has been difficulty bootstraping Kargo with a default Project already in place:

  • If the Project is applied before the webhook registration, Project creation succeeds, but creation of the underlying namespace does not take place. If the manifests contain additional resources that belong in that namespace, their creation will fail.

  • If the webhook registration is applied before the Project, Project creation will fail since the webhook server isn't running yet. Creation of the underlying namespace does not take place and, as above, if the manifests contain additional resources that belong in that namespace, their creation will fail.

Some unsatisfactory solutions we explored included:

  • Ensuring the webhook server is ready before attempting to create the Project.

    This was deemed to be difficult, or at least annoying.

  • "Create projects the hard way" by directly creating an appropriately labeled namespace.

    This loses out on the benefits of having a Project resource. Those aren't significant now, but will be of increasing consequence over time.

We settled on partially rolling back #1431, allowing for a Project's underlying namespace to be created synchronously by a webhook or asynchronously by the management controller. This ability enables a better option:

  • "Create projects the hard way" by directly creating an appropriately labeled namespace, but avoid losing out on the additional benefits of having a proper Project resource by also defining that. It must precede registration of the webhook, but that's easy enough to ensure. When Kargo comes up, the Project resource will be reconciled and will "adopt" the corresponding namespace at that time (a process we're already using in the webhook since Allow Kargo to adopt existing namespaces #1443).

cc @gdsoumya

Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
@krancour krancour requested a review from a team as a code owner February 16, 2024 17:13
Copy link

netlify bot commented Feb 16, 2024

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
🔨 Latest commit 1f34303
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/65cf97b8a036ec0008e68e9f
😎 Deploy Preview https://deploy-preview-1482.kargo.akuity.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@krancour krancour self-assigned this Feb 16, 2024
@krancour krancour added this to the v0.4.0 milestone Feb 16, 2024
@krancour krancour changed the title make management controller able to create project namespaces again refactor: make management controller able to create project namespaces again Feb 16, 2024
Copy link
Contributor

@gdsoumya gdsoumya left a comment

Choose a reason for hiding this comment

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

LGTM

@krancour krancour added this pull request to the merge queue Feb 16, 2024
Merged via the queue into akuity:main with commit 0b0d148 Feb 16, 2024
19 of 20 checks passed
@krancour krancour deleted the krancour/ns-by-mgtm-controller branch February 16, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants