Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Provide hook to instruct Helm operator to refresh git mirrors #1776

Merged
merged 3 commits into from
Mar 7, 2019

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Feb 27, 2019

Fixes #1430

This adds a webhook to the Helm operator as described in the issue above. The webhook starts a goroutine which instructs every mirror to refresh.

@stefanprodan
Copy link
Member

stefanprodan commented Feb 28, 2019

I had the impression that fluxctl sync does a git sync and applies the manifests on the cluster. If that's the case I would expect that sync will also trigger helm upgrades for all releases after the git refresh.

@hiddeco
Copy link
Member Author

hiddeco commented Feb 28, 2019

@hiddeco hiddeco force-pushed the helm-op-git-sync-webhook branch 2 times, most recently from d461d1d to 9002e22 Compare February 28, 2019 12:54
@squaremo
Copy link
Member

Some method of authentication

In general it's desirable to defer to existing means of authentication, i.e., credentials for the Kubernetes API. However that requires either more work on the part of the user (to construct a port forward or proxy) or set up a reverse proxy/ingress (with its own authentication); or a command-line tool for the helm operator, which we don't have.

I guess we could have fluxctl helm notify or some such, which would use the port-forwarding already in fluxctl.

@hiddeco
Copy link
Member Author

hiddeco commented Feb 28, 2019

I guess we could have fluxctl helm notify or some such, which would use the port-forwarding already in fluxctl.

I am somewhat opposed to this as it tightens the relationship between the 3 tools currently living in this repository.

fluxctl and the daemon already have a strong relationship but with our plans for the operator to actually move away from this repository I am afraid this would be a step into the wrong direction.

@stefanprodan
Copy link
Member

stefanprodan commented Feb 28, 2019

I think most users will be happy with:

kubectl -n flux port-forward deployment/flux-helm-operator 3030:3030 &
curl -XPOST http://localhost:3030/api/v1/sync-git

@squaremo
Copy link
Member

fluxctl and the daemon already have a strong relationship but with our plans for the operator to actually move away from this repository I am afraid this would be a step into the wrong direction.

Yeah, it is a weak suggestion. Leaving it to people to construct a port-forward themselves is probably acceptable.

@hiddeco hiddeco marked this pull request as ready for review March 1, 2019 12:04
@hiddeco hiddeco requested a review from squaremo March 4, 2019 13:00
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

I'm OK with this as it is (OK, maybe with typos excised), but please consider my suggestion in the comments.

site/helm-integration.md Outdated Show resolved Hide resolved
site/helm-integration.md Outdated Show resolved Hide resolved
integrations/helm/chartsync/chartsync.go Outdated Show resolved Hide resolved
integrations/helm/http/daemon/server.go Outdated Show resolved Hide resolved
integrations/helm/http/daemon/server.go Outdated Show resolved Hide resolved
@hiddeco hiddeco force-pushed the helm-op-git-sync-webhook branch 3 times, most recently from 9243ff5 to 7b58d38 Compare March 4, 2019 17:43
@hiddeco hiddeco requested a review from squaremo March 6, 2019 23:38
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Looks good Hidde 👍

Todo:
- put some authentication (simple user configured key?) in front of
  API route
- make it pretty
- tests
@hiddeco hiddeco merged commit ae751db into master Mar 7, 2019
@hiddeco hiddeco deleted the helm-op-git-sync-webhook branch March 7, 2019 14:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants