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

Allow creation of ArgoCDs and AppProjects #151

Merged
merged 2 commits into from
Feb 29, 2024
Merged

Allow creation of ArgoCDs and AppProjects #151

merged 2 commits into from
Feb 29, 2024

Conversation

mhutter
Copy link
Contributor

@mhutter mhutter commented Feb 27, 2024

While this component already managed the ArgoCD operator and the Syn ArgoCD instance, additional ArgoCD instances could only be deployed as adhoc-manifests. While this works, it is not possible to leverage Syn features such as hierarchical configuration, resulting in quite some duplication.

This commit introduces the new parameters argocds and projects, which allow the creation of ArgoCD and AppProject instances respectively.

In general, the component tries to be as un-intrusive as possible, so we only set minimal default values:

For ArgoCD: spec.applicationInstanceLabelKey - if this value collides for two ArgoCD instances on the same cluster, ArgoCD cannot properly determine which resources belong to "itself", and might try to prune resources managed by other ArgoCD instances.

Checklist

  • The PR has a meaningful title. It will be used to auto-generate the
    changelog.
    The PR has a meaningful description that sums up the change. It will be
    linked in the changelog.
  • PR contains a single logical change (to build a better changelog).
  • Update the documentation.
  • Categorize the PR by adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog.
  • Link this PR to related issues or PRs.

@mhutter mhutter added the enhancement New feature or request label Feb 27, 2024
@mhutter mhutter self-assigned this Feb 27, 2024
@mhutter
Copy link
Contributor Author

mhutter commented Feb 27, 2024

I'll reuse an existing golden-test to reduce the amount of crap in here

@mhutter mhutter marked this pull request as draft February 27, 2024 16:35
@mhutter mhutter changed the title feat: Allow creation of arbitrary ArgoCD instances feat: Allow creation of ArgoCDs and AppProjects Feb 27, 2024
While this component already managed the ArgoCD operator and the Syn
ArgoCD instance, additional ArgoCD instances could only be deployed as
adhoc-manifests. While this works, it is not possible to leverage Syn
features such as hierarchical configuration, resulting in quite some
duplication.

This commit introduces the new parameters `argocds` and `projects`,
which allow the creation of `ArgoCD` and `AppProject` instances
respectively.

In general, the component tries to be as un-intrusive as possible, so we
only set minimal default values:

For `ArgoCD`: spec.applicationInstanceLabelKey` - if this value collides
for two ArgoCD instances on the same cluster, ArgoCD cannot properly
determine which resources belong to "itself", and might try to prune
resources managed by other ArgoCD instances.

Signed-off-by: Manuel Hutter <manuel@hutter.io>
@mhutter mhutter marked this pull request as ready for review February 27, 2024 17:06
Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

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

LGTM overall. Some inline comments for potential larger improvements, but I have no strong opinion either way.

@mhutter
Copy link
Contributor Author

mhutter commented Feb 28, 2024

Thanks for the input @simu, I have updated the PR

As mentioned in the code review, AppProject resources created usig this
component usually only exist within the context of an ArgoCD instance
ALSO deployed using this component, so it makes sense to nest these
resources.

As a side-effect, this allows me to use `instances` as the parameter
key, which I prefer :-)

Additionally we now validate that no two instances are deployed to the
same namespace.

Signed-off-by: Manuel Hutter <manuel@hutter.io>
@mhutter mhutter mentioned this pull request Feb 28, 2024
5 tasks
@simu simu changed the title feat: Allow creation of ArgoCDs and AppProjects Allow creation of ArgoCDs and AppProjects Feb 29, 2024
Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

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

LGTM now. The feat: prefix in the PR title is unnecessary and would be added verbatim to the changelog in the release notes. Setting the enhancement label is sufficient to mark the change as a new feature/enhancement.

@mhutter mhutter merged commit 80099da into master Feb 29, 2024
10 checks passed
@mhutter mhutter deleted the feat/instances branch February 29, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants