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

Migrate from fluxcd-community charts to Flux-Operator #166

Merged
merged 25 commits into from
Jun 17, 2024

Conversation

kingdonb
Copy link
Contributor

@kingdonb kingdonb commented Jun 13, 2024

This is close enough to what we discussed at the Cozystack meeting today, to push as a draft

The flux-operator chart is much simpler

  • I will test upgrade locally on my home lab cozystack ☑️

I think we can just use kubectl apply twice, and avoid creating a second "flux-instances" package in core for now (edit: it worked!)

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
we need to conform to the current version in use, 2.2.x, but otherwise
we don't need to specify all of this, and as long as we don't specify
any of this, then user can merge their own configs in with kubectl apply

I hope the FluxInstance registry is set to default to ghcr, so we don't
block anyone from using an enterprise or paid distribution of flux with
this config!

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
spec.distribution.registry: Required value
(drat!)

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
@kingdonb
Copy link
Contributor Author

kingdonb commented Jun 13, 2024

At this point I have tested it out locally enough to know that we need at least spec.distribution.registry in the FluxInstance - which is a shame, we'll have to figure out another way that users can add their own settings for registry here, in case they bought Flux Enterprise ;)

Otherwise, users should be able to simply edit the FluxInstance in-place and have their edits merged in, I'll begin testing that. The migration was a success, the first thing that I expected happened once I corrected the error in the FluxInstance mentioned above, was the two image controllers got torn down. But that didn't happen - This is a default configuration in Flux, they are "extra controllers"

I can try using this for a while to see if there are any issues, if it enables all of the customizability that I was hoping, give it a few install-tests, but unless we uncover something I haven't already seen, this is going to be a very easy transition!

@kingdonb kingdonb marked this pull request as ready for review June 13, 2024 18:05
@kingdonb kingdonb changed the title WIP: Flux operator Migrate from fluxcd-community charts to Flux-Operator Jun 13, 2024
name: flux
spec:
distribution:
version: 2.2.x
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pinned the version 2.2.x here because I imagine we will want to pin some version, and I know the 2.2.x version is the one that's currently used. We may not need any migration for the 2.3 upgrade because Kubernetes API does the upgrade transparently, from v2beta2 to v2, without intervention.

But these templates will need to be changed at least:

% egrep -ir helm.toolkit.fluxcd.io/v2beta2 *
packages/core/platform/templates/helmreleases.yaml:apiVersion: helm.toolkit.fluxcd.io/v2beta2
packages/core/platform/templates/apps.yaml:{{- if .Capabilities.APIVersions.Has "helm.toolkit.fluxcd.io/v2beta2" }}
packages/core/platform/templates/apps.yaml:{{- $tenantRoot = lookup "helm.toolkit.fluxcd.io/v2beta2" "HelmRelease" "tenant-root" "tenant-root" }}
packages/core/platform/templates/apps.yaml:apiVersion: helm.toolkit.fluxcd.io/v2beta2

I left that stuff alone and opted for the version pin instead, we can upgrade Flux later, or ideally before the next release so we are caught up.

It should be easy enough to test the upgrade now, I guess I can build that out in a separate PR (and try to confirm if we don't really need any migrations!)

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, thank you

@kingdonb

This comment was marked as outdated.

the chart installs by default all 6 components, so make sure we are
taking them all over this way

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
@kingdonb
Copy link
Contributor Author

(I added them to the components list, that worked)

@kingdonb kingdonb mentioned this pull request Jun 13, 2024
Copy link
Member

@kvaps kvaps left a comment

Choose a reason for hiding this comment

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

Now we have chicken-and-egg problem, Fluxcd can't be installed during the initial installation as CNIs are not installed yet, but CNI can't be installed because this construction expects that HelmRelease CR will be existing in a cluster

install_basic_charts() {
if [ "$BUNDLE" = "paas-full" ] || [ "$BUNDLE" = "distro-full" ]; then
make -C packages/system/cilium apply resume
fi
if [ "$BUNDLE" = "paas-full" ]; then
make -C packages/system/kubeovn apply resume
fi
}

apply: suspend ## Apply Helm release to a Kubernetes cluster
kubectl get hr -n $(NAMESPACE) $(NAME) -o jsonpath='{.spec.values}' | helm upgrade -i -n $(NAMESPACE) $(NAME) . -f -

here is part of my cozystack log:

make: Leaving directory '/cozystack/packages/core/platform'
make: Entering directory '/cozystack/packages/core/fluxcd'
helm template -n cozy-fluxcd fluxcd . --no-hooks --dry-run=server -a admissionregistration.k8s.io/v1 -a apiextensions.k8s.io/v1 -a apiregistration.k8s.io/v1 -a apps/v1 -a authentication.k8s.io/v1 -a authorization.k8s.io/v1 -a autoscaling/v1 -a autoscaling/v2 -a batch/v1 -a certificates.k8s.io/v1 -a coordination.k8s.io/v1 -a discovery.k8s.io/v1 -a events.k8s.io/v1 -a flowcontrol.apiserver.k8s.io/v1 -a flowcontrol.apiserver.k8s.io/v1beta3 -a fluxcd.controlplane.io/v1 -a networking.k8s.io/v1 -a node.k8s.io/v1 -a policy/v1 -a rbac.authorization.k8s.io/v1 -a scheduling.k8s.io/v1 -a storage.k8s.io/v1 -a v1 | kubectl apply -n cozy-fluxcd -f-
serviceaccount/fluxcd-flux-operator unchanged
customresourcedefinition.apiextensions.k8s.io/fluxinstances.fluxcd.controlplane.io unchanged
clusterrolebinding.rbac.authorization.k8s.io/fluxcd-flux-operator unchanged
service/fluxcd-flux-operator unchanged
deployment.apps/fluxcd-flux-operator configured
kubectl apply -n cozy-fluxcd -f flux-instance.yaml
fluxinstance.fluxcd.controlplane.io/flux unchanged
make: Leaving directory '/cozystack/packages/core/fluxcd'
error: the server doesn't have a resource type "helmrepositories"
time="2024-06-15T15:17:59Z" level=fatal msg="failed to run" err="exit status 1"

@kvaps
Copy link
Member

kvaps commented Jun 15, 2024

I was wrong, it seems happening right on the next step, here

kubectl annotate helmrepositories.source.toolkit.fluxcd.io -A -l cozystack.io/repository reconcile.fluxcd.io/requestedAt=$(date +"%Y-%m-%dT%H:%M:%SZ") --overwrite

@kingdonb
Copy link
Contributor Author

Perhaps we can add another wait step - the Flux-Operator FluxInstance has a ready status, let me take a look...

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
@kingdonb
Copy link
Contributor Author

We could add the wait in the package itself, or in the installer script. I'll give it a local test, I built a new installer image here and I have a little bit of time I can try wiping and reloading the cluster, and follow the logs 👍

Thanks for the review!

@kingdonb
Copy link
Contributor Author

Waiting where I have placed it makes this fail... I'll work it out and let you know when it's worked out here

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
IDK

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
name: flux
spec:
cluster:
domain: cozy.local
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to not hardcode the domain? Users might use the different one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can make our own local values in the parent chart, and keep them for the FluxInstance - sure, I think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything that happens in the chart now is either a parameter or an override. 🎸 🚀 🪨

Copy link
Contributor Author

@kingdonb kingdonb Jun 16, 2024

Choose a reason for hiding this comment

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

Maybe we can figure out how to pass-through to accept the domain as a parameter from the cozystack ConfigMap, before this merges - this will help me know how to add the distribution.registry later in a future build 👍

Copy link
Member

Choose a reason for hiding this comment

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

@stefanprodan provided an elegant solution for removing cluster domain dependency from fluxcd:
controlplaneio-fluxcd/flux-operator#34 (comment)

I'd like to go this way right now as we won't break the exisiting user flow.

But it seems we still have to add cluster-domain and kubernetes-endpoint options into cozystack configmap.
So if specified, it must be used by fluxcd and other applications, because in some cases this might be required.

I'll prepare an issue to keep this in mind.

Copy link
Member

Choose a reason for hiding this comment

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

done #168

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
it's tested now, it works, we know where the failures are (and they
aren't a problem for now)

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
@kingdonb
Copy link
Contributor Author

kingdonb commented Jun 16, 2024

I'm going to move 6d71499 (now e51f17b) to #167 and rebase that one on this one.

This is tested now, no errors and it works in a fresh install. It could use some upgrade testing, I think, but should also work. The upgrade was the first thing I tested.

@kingdonb
Copy link
Contributor Author

kingdonb commented Jun 16, 2024

I think that #167 is mergeable now, or this one.

But be aware that flux-operator chart is out of sync with upstream, as these PRs haven't merged yet:

They will take another release of flux-operator, which won't happen before Monday anyway, so I suggest we can wait for it then we can run make update so the version numbers are in sync (and we are not making a fork later, in case something over there doesn't merge as-is.)

Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
@stefanprodan
Copy link

You may want to wait for controlplaneio-fluxcd/flux-operator#33 to get automated updates going forward. I will do a release with this next week, then you can add distribution.artifact to your FluxInstance template.

@kvaps kvaps changed the base branch from main to upd-flux June 17, 2024 13:57
@kvaps kvaps merged commit 54017b6 into aenix-io:upd-flux Jun 17, 2024
This was referenced Jun 17, 2024
@kingdonb kingdonb deleted the flux-operator branch June 18, 2024 11:33
kingdonb added a commit to kingdonb/cozystack that referenced this pull request Jun 22, 2024
Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
kvaps added a commit that referenced this pull request Jun 24, 2024
This cumulative PR includes the following changes:

- Migrate from fluxcd-community charts to Flux-Operator #166
- Upgrade to Flux 2.3.x #167
- Refactor Flux 2.3 update #172
- Update flux plugin for dashboard #171
- Flux Operator 0.6 #178
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.

None yet

3 participants