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

Flux v2 support #3066

Merged
merged 9 commits into from Feb 3, 2021
Merged

Flux v2 support #3066

merged 9 commits into from Feb 3, 2021

Conversation

Callisto13
Copy link
Contributor

@Callisto13 Callisto13 commented Jan 12, 2021

please nobody squash-merge this. i want small commits for this otherwise future us will be mad

also nobody click "Update branch". i hate that thing


TLDR, Files to look at:

New stuff:

  • examples/12-gitops-toolkit.yaml
  • pkg/actions/flux/enable.go
  • pkg/actions/flux/enable_test.go
  • pkg/apis/eksctl.io/v1alpha5/defaults.go
  • pkg/apis/eksctl.io/v1alpha5/types.go
  • pkg/ctl/cmdutils/configfile.go
  • pkg/ctl/cmdutils/gitops.go
  • pkg/ctl/enable/flux.go
  • pkg/executor/executor.go
  • pkg/flux/flux.go
  • pkg/flux/flux_test.go
  • pkg/gitops/gitops.go
  • userdocs/src/usage/gitops.md
  • pkg/ctl/enable/flux_test.go

Reorganised/Refactored old stuff:

  • pkg/actions/repo/enable.go
  • pkg/actions/repo/enable_test.go
  • pkg/actions/repo/wait.go
  • pkg/ctl/enable/repo.go
  • pkg/git/git_test.go

Description

This PR lets users define Flux v2 (aka gitops toolkit) configuration in their cluster config. The config can be applied in one of two commands:

eksctl enable flux -f <file>  # install toolkit to existing cluster
eksctl create flux -f <file>  # flux2 will be installed in a new cluster after creation

As discussed in planning, I have introduced a new subcommand to enable because the flags for v2 in no way line up with those in v1, therefore simply adding a --use-v2 style flag to the existing subcommand would have given a rubbish UX.

Edit: We landed on creating a new top-level config object since the old git. paths are soon to become redundant and have no relevance in the current plans for GitOps. gitops gives us room to add more toolkit options underneath later, and means we can cleanly build alongside git codepaths without interaction which will make removing things easier. There may be some confusion for users ("was the previous config not flux/gitops?") but hopefully we can counter that will docs.

Implementation-wise, I went with simply shelling out to Flux. Ensuring that the binary is there is something we already insist on for the aws-cli and kubectl. Also, the Flux code doesn't wrap their bootstrap functionality in a library, and copy-pasting ~300 lines from there to here is worse.


Flags:

There are no additional flags for this command (beyond --config-file). The reasons for this are:

  1. Unlike Flux v1, bootstrapping Flux v2 components is very simple: just one command. The value in eksctl providing flags for v1 is that it turned a multi-step process into just 1 for users. There is no need for users to run one command with eksctl, with flags delegated to flux when they can just use flux directly. The value eksctl delivers here is in allowing users to specify a config file with the possibility of having it run automatically after a create, so that is what I have implemented. (If users want flags later we can do that for them maybe.)
  2. We are moving towards a config-only world soon, and the project has been trying to avoid adding new flags for a while. With this command it seems to make sense to follow this.

Config:

I have implemented the bare minimum of Flux2 config options and left many things to default in this first pass. We can expand and add more features as we need them. Whether we intend to 1:1 pass all config options through is a discussion for another time.

The config file looks like this:

gitops:
  flux:
    gitProvider: github             # required. options are github or gitlab
    owner: dr-who                   # required
    repository: my-cluster-gitops   # required
    personal: true                  # optional. if left false, assumes 'owner' is an org
    branch: main                    # optional
    namespace: "flux-system"        # optional
    path: "clusters/cluster-27"     # optional

Refactoring:

I have moved some of the Flux v1 (aka repo) stuff around so that a) it fits with the actions organisation and b) the implementation sits alongside the new interface (I know it doesn't need to but it is nice).

I have done some minimal refactoring and fixing of existing tests, but kept it to a minimum since the diff was already quite large (and that code will all be gone by the summer anyway).


Still to come in other PRs:

  • Integration tests
  • Various missing improvements and configuration which has been deliberately left for another day

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup), target version (e.g. version/0.12.0) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@Callisto13 Callisto13 linked an issue Jan 12, 2021 that may be closed by this pull request
@Callisto13 Callisto13 force-pushed the cb-flux-v2 branch 3 times, most recently from 6a22b14 to add5f56 Compare January 13, 2021 12:28
@Callisto13 Callisto13 added the kind/feature New feature or request label Jan 13, 2021
@Callisto13 Callisto13 changed the title WIP: Flux V2 integration Flux v2 support Jan 13, 2021
@Callisto13 Callisto13 marked this pull request as ready for review January 13, 2021 13:10
@Callisto13
Copy link
Contributor Author

Callisto13 commented Jan 13, 2021

./eksctl enable flux -f test.yaml
[!]  running experimental command
[ℹ]  eksctl version 0.37.0-dev+b001ff82.2021-01-13T13:16:19Z
[ℹ]  will install Flux v2 components on cluster cb-flux-test
[ℹ]  ensuring v1 repo components not installed
[ℹ]  checking whether toolkit components already installed
[!]  helm-controller deployment was not found
[!]  kustomize-controller deployment was not found
[!]  notification-controller deployment was not found
[!]  source-controller deployment was not found
[ℹ]  running pre-flight checks
► checking prerequisites
✔ kubectl 1.19.4 >=1.18.0
✔ Kubernetes 1.18.9-eks-d1db3c >=1.16.0
✔ prerequisites checks passed
[ℹ]  bootstrapping gitops toolkit into cluster
► connecting to github.com
✔ repository created
✔ repository cloned
✚ generating manifests
✔ components manifests pushed
► installing components in flux-system namespace
namespace/flux-system created
networkpolicy.networking.k8s.io/allow-scraping created
networkpolicy.networking.k8s.io/allow-webhooks created
networkpolicy.networking.k8s.io/deny-ingress created
role.rbac.authorization.k8s.io/crd-controller-flux-system created
rolebinding.rbac.authorization.k8s.io/crd-controller-flux-system created
clusterrolebinding.rbac.authorization.k8s.io/cluster-reconciler-flux-system created
customresourcedefinition.apiextensions.k8s.io/buckets.source.toolkit.fluxcd.io configured
customresourcedefinition.apiextensions.k8s.io/gitrepositories.source.toolkit.fluxcd.io configured
customresourcedefinition.apiextensions.k8s.io/helmcharts.source.toolkit.fluxcd.io configured
customresourcedefinition.apiextensions.k8s.io/helmrepositories.source.toolkit.fluxcd.io configured
service/source-controller created
deployment.apps/source-controller created
customresourcedefinition.apiextensions.k8s.io/kustomizations.kustomize.toolkit.fluxcd.io configured
deployment.apps/kustomize-controller created
customresourcedefinition.apiextensions.k8s.io/helmreleases.helm.toolkit.fluxcd.io configured
deployment.apps/helm-controller created
customresourcedefinition.apiextensions.k8s.io/alerts.notification.toolkit.fluxcd.io configured
customresourcedefinition.apiextensions.k8s.io/providers.notification.toolkit.fluxcd.io configured
customresourcedefinition.apiextensions.k8s.io/receivers.notification.toolkit.fluxcd.io configured
service/notification-controller created
service/webhook-receiver created
deployment.apps/notification-controller created
deployment "source-controller" successfully rolled out
deployment "kustomize-controller" successfully rolled out
deployment "helm-controller" successfully rolled out
deployment "notification-controller" successfully rolled out
✔ install completed
► configuring deploy key
✔ deploy key configured
► generating sync manifests
✔ sync manifests pushed
► applying sync manifests
◎ waiting for cluster sync
✔ bootstrap finished
[✔]  Flux v2 installed successfully
[✔]  see https://toolkit.fluxcd.io/ for usage instructions

@Callisto13 Callisto13 force-pushed the cb-flux-v2 branch 2 times, most recently from 51b2a0e to 93a3c67 Compare January 13, 2021 16:04
@stefanprodan
Copy link
Contributor

We do severals releases per week for flux2, unless eksctl plans to do the same to keep up with Flux changes I suggest we stick to using Flux CLI instead of a package. I don't expect users to just bootstrap a cluster, they will have to download Flux CLI anyway to build their GitOps pipelines.

@Callisto13
Copy link
Contributor Author

Callisto13 commented Jan 14, 2021

@stefanprodan We are using the CLI. We required users to have the CLI on the path, and then we shell out.

From the description:

Implementation-wise, I went with simply shelling out to Flux. Ensuring that the binary is there is something we already insist on for the aws-cli and kubectl. Also, the Flux code doesn't wrap their bootstrap functionality in a library, and copy-pasting ~300 lines from there to here is worse.

@stefanprodan
Copy link
Contributor

@Callisto13 maybe it's not clear from my comment, I did read the code changes in this PR and I'm in favour of using the Flux CLI. We could provide a bootstrap package but then eksctl would need to keep up with flux2 releases as well as Kubernetes.

@Callisto13
Copy link
Contributor Author

Sorry, morning brain!

So to clarify, you are saying to stick with the approach I have here because providing a library in Flux would in reality not be useful to us?

@stefanprodan
Copy link
Contributor

I have here because providing a library in Flux would in reality not be useful to us?

This is not about usefulness but about velocity.

pkg/gitops/gitops.go Outdated Show resolved Hide resolved
@Callisto13
Copy link
Contributor Author

@markramm are you happy with the subcommand on this?

@Callisto13
Copy link
Contributor Author

Doesn't have to be part of this PR, but we should add fluxcd/tap/flux to https://github.com/weaveworks/eksctl/blob/master/.goreleaser.brew.yml#L18

oh yes good catch 👍

@michaelbeaumont
Copy link
Contributor

@Callisto13 With fluxv2 do any of the 3 existing fields under git: make any sense? We can probably just have gitops top level

@Callisto13
Copy link
Contributor Author

agreed 👍 as discussed in standup and with feedback from Mark Ramm; we are gonna abandon the git top level object and have a new gitops object.

So the config would look like:

gitops:
  flux:
    gitProvider: github             # required. options are github or gitlab
    owner: dr-who                   # required
    repository: my-cluster-gitops   # required
    personal: true                  # optional. if left false, assumes 'owner' is an org
    branch: main                    # optional
    namespace: "flux-system"        # optional
    path: "clusters/cluster-27"     # optional

With the command being:

eksctl enable flux -f config.yaml

While enable flux may be confusing to users, it makes more sense in the context of a future Weave Gitops world. Eventually users would be able to call enable on other elements of WGO, or call enable gitops to install the entire configuration under that object.
The git object and everything below will be deprecated and removed along with the Repo/BootstrapProfiles implementation later in the year.

Gonna mark as a draft while I move over. I wont be changing any logic, just moving types around and renaming.

@Callisto13 Callisto13 marked this pull request as draft January 26, 2021 15:33
@Callisto13 Callisto13 marked this pull request as ready for review January 27, 2021 14:58
@Callisto13 Callisto13 force-pushed the cb-flux-v2 branch 2 times, most recently from cc3ddbe to 0d63fd4 Compare January 29, 2021 11:52
Base automatically changed from master to main February 2, 2021 15:06
Copy link
Contributor

@aclevername aclevername left a comment

Choose a reason for hiding this comment

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

LGTM

Part of the work to add Flux 2.
It makes more sense to have this outside the git package (which will
disappear with flux v1).

Having the ENV passed in as a map, and having the executor do the work,
is nicer than using an array.
Part of the work to add Flux 2.
This type will eventually supersede the existing Git type, scheduled to
be deprecated later this year.
A simple client to process the configuration into flags, and shell out
to the Flux binary.
Certain env vars need to be carried from the starting env to the command
env for Flux to work properly.
The library connecting the configuration to the execution. For now this
also verifies whether v1 components are already installed on the
cluster.
There are some minor alterations to existing Git code. This has been
done to avoid confusion. Those old pieces were built along dated plans
for gitops, which are soon to be removed.
Refactoring to bring in line with the plan to move everything into a
library.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flux v2 enable repo integration
4 participants