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

Multiple values for kargo.akuity.io/authorized-stage:<name> annotation #2971

Open
2 tasks done
mmclane opened this issue Nov 20, 2024 · 4 comments
Open
2 tasks done

Comments

@mmclane
Copy link

mmclane commented Nov 20, 2024

Checklist

  • I've searched the issue queue to verify this is not a duplicate feature request.
  • I've pasted the output of kargo version, if applicable.
  • [] I've pasted logs, if applicable.

Kargo v1.0.3

Proposed Feature

We should be able to specify multiple values in the kargo.akuity.io/authorized-stage: annotation so that multiple stages can trigger an ArgoCD update for the same Argo application.

Motivation

Some of our microservices is essentially made of of three (or more) things that need to be coordinated, a helm chart, a service container, and a microfrontend container. Today I have kargo project configured to assemble freight for these three things.
image

The problem with this approach is that later in my pipeline I need to create a release PR on the github repo associated with the artifact that is the source of the update. Kargo doesn't have any logic to say If this update is from subscription A use repoURL A, if from subscription B use repoURL B. So I don't have a way to create the PR in the correct repo.

I could however set the project up to have two separate warehouses with two separate pipelines. This approach actually might have a number of benefits but mostly it would allow me to create the PR in the correct repo.
image

The problem with the approach that there are two stages for each environment. This is ONLY a problem because they both need to tell ArgoCD to sync the same application. If I could have multiple values for kargo.akuity.io/authorized-stage: I would be able to make this work.

Suggested Implementation

I can think of a few possibilities:

  • Wildcards: kargo.akuity.io/authorized-stage: ':*-dev'
  • Comma Separated List: kargo.akuity.io/authorized-stage: ':,:'
  • Only specify project allows any stage in the project: kargo.akuity.io/authorized-stage: '' (not sure on this one)

If you ask me, I think the second option makes the most sense and is probably the easiest to implement.

@mmclane
Copy link
Author

mmclane commented Nov 20, 2024

For more information, there has been some discussion on all of this on the discord server today.

https://discord.com/channels/1138942074998235187/1138946346217394407/1308824098469580843

@krancour
Copy link
Member

This used to be allowed, actually, but was removed for two separate reasons:

  • When we transitioned away from the legacy promotion mechanisms to promotion steps, nearly all logic for integrating with Argo CD become isolated to just the argo-cd update step. The steps are mostly opaque to the rest of Kargo -- which is important since we're working toward allowing custom/third party steps.

    One aspect of the Argo CD integration that we unfortunately couldn't isolate in that step was the need for enqueuing a Stage to reconcile when the status of an Application it interacts with changes. This is important for a variety of reasons, not the least of which is having Stages notice health changes in the Applications promptly.

    With the legacy promotion mechanisms gone and configuration for all steps being, by design, opaque to the rest of Kargo, we'd lost the bit of information we'd been using to build a "Stages by App" index.

    In need of a new way to figure out what Stage needed to be enqueued for reconciliation when the an App status changed, we realized that kargo.akuity.io/authorized-stage had the exact information we need if not for the fact that it supported wildcards.

    I cannot recall the technical reason that we couldn't work out the connection with wildcards involved. It might have been that there was too much overhead involved in digging through all Stages in a cluster to see which ones matched the pattern. @hiddeco worked on this and could probably fill us in more accurately on what the technical constraint was there.

    Ref: feat(controller)!: queue Stages for Application changes using annotation #2617

    At the same time...

  • We had a number of issues reported in a short period of time where the ability for multiple Stages to update the same App proved to be a giant foot gun. There are some simple cases where one updates an Argo CD App and doesn't care to continuously verify that the App is observably synced to the correct revisions of things. (Stages develop their notion of the correct revisions of things from their current collection of Freight.) This isn't the average case, however. In general, people care about that.

    And herein lies the foot gun. If one Stage forces an App to sync and expects to observe it synced to revision A and a second Stage forced the same App to sync and expects to observed it synced to revision B, there's now a conflict. One of these Stages is going to appear unhealthy because they don't agree on what the App should look like. We saw a lot of people creating trouble for themselves in this manner.

    With this being a frequent issue, giving up the wildcard support to solve the other issue started to look like a win-win.

So with all this being said, putting the wildcard support back isn't a decision we should rush into or take lightly.

But there is good news.

  1. The v1.1 release is just around the corner and it adds support for expressions. Although these won't, on their own, solve your problem, adding support for expressions means support for conditional steps cannot be far behind. This could allow you to interact with different git repositories depending on context. As that would solve your original issue, this issue may end up as a road you don't need to go down.

  2. Although the thread went off on a bit of a tangent, Removing desiredCommitFromStep: commit from argocd-update causes infinite sync loop #2952, once addressed, will change the notion of how or even if a Stage develops a sense of what specific revision(s) Apps should be synced to. Since that will essentially make factoring an Apps revisions into Stage health an opt-in the foot gun I alluded to is mostly removed. If we were to seriously consider putting the wildcard support back (if it proves technically possible, of course) then that is an easier notion to entertain after that foot gun is gone.

Last, but not least, returning to your original issue:

The problem with this approach is that later in my pipeline I need to create a release PR on the github repo associated with the artifact that is the source of the update. Kargo doesn't have any logic to say If this update is from subscription A use repoURL A, if from subscription B use repoURL B. So I don't have a way to create the PR in the correct repo.

This seems odd. I often think of promotion processes as "mashing up" the multiple artifacts into a piece of Freight to arrive at some new state. Why would the recipe for the new state be different if A is the new artifact vs B vs C?

I call this out not because I think you're doing anything objectively wrong, but as I've never seen something like that before, it's natural to wonder if there wasn't a simpler option that was overlooked, in which case, we'd be happy to try and give some further guidance.

@hiddeco
Copy link
Contributor

hiddeco commented Nov 20, 2024

I cannot recall the technical reason that we couldn't work out the connection with wildcards involved. It might have been that there was too much overhead involved in digging through all Stages in a cluster to see which ones matched the pattern.

This is indeed the precise issue. With a wild card, any Application could and/or would be mapped to any Stages of any Projects without there necessarily being a relationship. As you do not want to request an unnecessary reconciliation for all of these objects, it would require even deeper inspection and special treatment of the argocd-update step.

Having an exact 1:1 mapping was the easiest solution to prevent this, as it is simple and explicit.

@krancour
Copy link
Member

Thanks @hiddeco.

Although it's not the exact ask here, I do remain optimistic that we are trending toward resolving the issues that would make someone want to manage one App with multiple Stages in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants