Skip to content

Refactor Flux 2.3 update - #172

Merged
Andrei Kvapil (kvaps) merged 1 commit into
upd-fluxfrom
flux-2.3-refactor
Jun 24, 2024
Merged

Refactor Flux 2.3 update#172
Andrei Kvapil (kvaps) merged 1 commit into
upd-fluxfrom
flux-2.3-refactor

Conversation

@kvaps

@kvaps Andrei Kvapil (kvaps) commented Jun 17, 2024

Copy link
Copy Markdown
Member

This PR:

@kvaps
Andrei Kvapil (kvaps) changed the base branch from main to upd-flux June 17, 2024 14:12
@kvaps Andrei Kvapil (kvaps) changed the title flux 2.3 refactor refactor Flux 2.3 update Jun 17, 2024
@kvaps Andrei Kvapil (kvaps) changed the title refactor Flux 2.3 update Refactor Flux 2.3 update Jun 17, 2024
@kvaps Andrei Kvapil (kvaps) mentioned this pull request Jun 17, 2024
5 tasks
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
@kingdonb

Copy link
Copy Markdown
Member

I'm checking out this PR now. I see that flux-operator 0.6 is out, a few hours ago. I'll run make update and see what divergences exist between the upstream chart and the local clone. Ideally the changes having been merged upstream will allow us to remove any patches we've added out-of-band to the flux-operator chart.

I saw you merged the values files together, I don't know how to support this idea, but I was hoping we could allow users to select their own values somehow, maybe through a toggle. The idea being that the more fields we set in our FluxInstance the more difficult it will be for users to customize it.

In the earlier PR of the two that got merged, #166 I think I kept it minimal, and then added more detailed customization in #167 - in my initial idea, ideally the whole spec.kustomize block would be left unset, so users can 3-way merge in their own configuration there.

Then on second thought I guess it makes more sense if cozystack is creating the FluxInstance to expose a configuration opportunity for the users more directly, they shouldn't have to know about 3-way merge behavior just to configure Flux!

@kingdonb

Kingdon Barrett (kingdonb) commented Jun 22, 2024

Copy link
Copy Markdown
Member

The network policy appears a bit messed up.

allow-from-dashboard is present in cozy-fluxcd ns, but the usual flux networkpolicy resources aren't present (so the default deny rule that Flux installs is not the issue)

The network patch (alternative to setting cluster.domain) didn't work for me. It correctly identified the service to poll (source-controller) and it looks like coreDNS is directing us to the right address, but it's getting I/O timeout. The error:

cozy-cilium                      cilium                      6d    False   Could not load chart: GET http://source-controller.cozy-fluxcd.svc/helmchart/cozy-system/cozy-cilium-cilium/cozy-cilium-0.1.0.tgz giving up after 10 attempt(s): Get "http://source-controller.cozy-fluxcd.svc/helmchart/cozy-system/cozy-cilium-cilium/cozy-cilium-0.1.0.tgz": dial tcp 10.96.254.142:80: i/o timeout

This may have been the cilium issue you were talking about before?

@kingdonb

Copy link
Copy Markdown
Member

Removing the network policy that cozystack imposes (I can't find where, I did spot it earlier) allows flux pods to talk again:

cozy-fluxcd      allow-from-dashboard              <none>                                                                                                          40m

I suspect that flux would be talking fine if the default network policies weren't skipped. I'm not sure why they don't apply. I'll keep poking around, I have not yet mastered the build process for cozystack + all its parts

@kingdonb

Copy link
Copy Markdown
Member

I see that allow-from-dashboard policy has actually been removed in d44681b

The flux network policies weren't created by default because:

looks like a bug in the operator, it can be easily worked around by setting any of the values under cluster. so that networkPolicy: true default is set. (I hadn't seen it before because I've been setting cluster.domain)

What was the purpose of removing allow-from-dashboard ? (it may need to be re-added if this bug is fixed upstream.)

@kingdonb

Copy link
Copy Markdown
Member

remove policy to allow access fluxcd from dashboard because now fluxcd shipped without policies

I'm sure this is a bug, networkPolicy: true is documented as a default, but we're not getting that value because we haven't set any values under cluster. Cozystack isn't setting up any secrets in flux-system to protect for now, so maybe we can keep this off, but I don't think it's intentional that these network policies are not shipped by default anymore.

I opened:

| commonAnnotations | object | `{}` | Common annotations to add to all deployed objects including pods. |
| commonLabels | object | `{}` | Common labels to add to all deployed objects including pods. |
| fullnameOverride | string | `""` | |
+| hostNetwork | bool | `false` | If `true`, start flux-operator in hostNetwork mode. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is not in sync with the upstream chart, please copy https://github.com/controlplaneio-fluxcd/charts/tree/main/charts/flux-operator

@kingdonb Kingdon Barrett (kingdonb) Jun 22, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have make update which pulls from OCI chart upstream, and for flux-operator 0.6 - #178 will bring it back in sync, before these changes are released. I already updated in that PR to fix fuzz in one of the patches here, and deleted the other one for hostNetwork that is no longer needed because it got merged upstream in controlplaneio-fluxcd/charts#10.

@kvaps
Andrei Kvapil (kvaps) merged commit b8e5309 into upd-flux Jun 24, 2024
@kvaps
Andrei Kvapil (kvaps) deleted the flux-2.3-refactor branch June 24, 2024 11:14
Andrei Kvapil (kvaps) added a commit that referenced this pull request Jun 24, 2024
This PR upgrades to Flux-Operator 0.6 released this morning, also includes:

* #170
which is an aggregate PR, so #171 #172 etc. I think this PR now basically subsumes #170 and can replace it.

I have at least 80% confidence there are no errors in this PR. It also restores the networkPolicy default and the deleted cozy-dashboard network policy, which we will see fixed (restored to install NetworkPolicy resources by default) in the next `flux-operator` release.

Ref: controlplaneio-fluxcd/flux-operator#52
Andrei Kvapil (kvaps) pushed a commit that referenced this pull request Jun 24, 2024
This PR upgrades to Flux-Operator 0.6 released this morning, also includes:

* #170
which is an aggregate PR, so #171 #172 etc. I think this PR now basically subsumes #170 and can replace it.

I have at least 80% confidence there are no errors in this PR. It also restores the networkPolicy default and the deleted cozy-dashboard network policy, which we will see fixed (restored to install NetworkPolicy resources by default) in the next `flux-operator` release.

Ref: controlplaneio-fluxcd/flux-operator#52
Andrei Kvapil (kvaps) pushed a commit that referenced this pull request Jun 24, 2024
This PR upgrades to Flux-Operator 0.6 released this morning, also includes:

* #170
which is an aggregate PR, so #171 #172 etc. I think this PR now basically subsumes #170 and can replace it.

I have at least 80% confidence there are no errors in this PR. It also restores the networkPolicy default and the deleted cozy-dashboard network policy, which we will see fixed (restored to install NetworkPolicy resources by default) in the next `flux-operator` release.

Ref: controlplaneio-fluxcd/flux-operator#52
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Andrei Kvapil (kvaps) pushed a commit that referenced this pull request Jun 24, 2024
This PR upgrades to Flux-Operator 0.6 released this morning, also includes:

* #170
which is an aggregate PR, so #171 #172 etc. I think this PR now basically subsumes #170 and can replace it.

I have at least 80% confidence there are no errors in this PR. It also restores the networkPolicy default and the deleted cozy-dashboard network policy, which we will see fixed (restored to install NetworkPolicy resources by default) in the next `flux-operator` release.

Ref: controlplaneio-fluxcd/flux-operator#52
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Andrei Kvapil (kvaps) pushed a commit that referenced this pull request Jun 24, 2024
This PR upgrades to Flux-Operator 0.6 released this morning, also includes:

* #170
which is an aggregate PR, so #171 #172 etc. I think this PR now basically subsumes #170 and can replace it.

I have at least 80% confidence there are no errors in this PR. It also restores the networkPolicy default and the deleted cozy-dashboard network policy, which we will see fixed (restored to install NetworkPolicy resources by default) in the next `flux-operator` release.

Ref: controlplaneio-fluxcd/flux-operator#52
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Andrei Kvapil (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.

3 participants