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

Alerts/Github - is it possible to separate add additional metadata? #589

Open
audunsolemdal opened this issue Jul 31, 2023 · 7 comments
Open

Comments

@audunsolemdal
Copy link

So my goal is to run a github action step to check if fluxcd reconciliation for a kustomization towards a specificcluster was successful. There are three different kustomizations in three different clusters which may post reconcilation successful status to my commit on github. The kustomizations have the same exact name It does not seem like modifying the eventMetadata or summary fields in the Alert resource adds any extra metadata to the github status.

I guess a workaround is to name the kustomizations tstsp1-github-info-${cluster} rather than just tstsp1-github-info, but I am wondering if there are other methods to inject this metadata into the Github status? The description or context fields would be sufficient to modify in my use case.

Say I have a repo with the following structure

orgX/wl-tstsp1:

fluxcd/
  dev/
    deployment.yaml
  test/
    deployment.yaml
  prod/
    deployment.yaml

Each folder is connected to the namespace called tstsp1 in three different cluster (dev/test/prod)
From each of the clusters there is a alert updating the github status

---
apiVersion: notification.toolkit.fluxcd.io/v1beta2
kind: Provider
metadata:
  name: tstsp1-github
  namespace: flux-system
spec:
  type: github
  address: https://github.com/orgX/wl-tstsp1
  secretRef:
    name: gh-flux-pat
---
apiVersion: notification.toolkit.fluxcd.io/v1beta2
kind: Alert
metadata:
  name: "tstsp1-github-info"
  namespace: flux-system
spec:
  providerRef:
    name: tstsp1-github
  eventSeverity: info
  summary: "dev01"
  eventMetadata:
    cluster: dev01
  eventSources:
    - kind: Kustomization
      name: '*'
      namespace: tstsp1

I end up with a status looking somewhat like the below:

gh api /repos/orgX/wl-tstsp1/statuses/88efe1eeb3e2ad9724acc6ff7c06324f6e613a66
[
  {
    "url": "https://api.github.com/repos/orgX/wl-tstsp1/statuses/88efe1eeb3e2ad9724acc6ff7c06324f6e613a66",
    "avatar_url": "https://avatars.githubusercontent.com/u/18458716?v=4",
    "id": 24342874371,
    "node_id": "SC_kwDOHpAnes8AAAAFqvLJAw",
    "state": "success",
    "description": "reconciliation succeeded",
    "target_url": null,
    "context": "kustomization/tstsp1/025ecd77",
    "created_at": "2023-07-31T10:38:27Z",
    "updated_at": "2023-07-31T10:38:27Z",
    "creator": {
      ...
    }
  }
]   
@makkes
Copy link
Member

makkes commented Aug 1, 2023

The context field of the status is constructed from the involved object and the UID of the provider (.metadata.uid of each Provider object) to make it unique across clusters but I reckon you might want something more easily trackable to the cluster the status came from.

The commit status API accepts the parameters state, description, context and target_url. The first three are already used by Flux but maybe we can make the context configurable to include an additional field derived from Alert.spec.Summary (which ends up in the event metadata).

@audunsolemdal
Copy link
Author

I'll be happy with any solution that doesn't involve renaming my kustomizations per cluster.
I also tried setting

eventMetadata:
  context: ${cluster}

to no avail, my understanding is that certain fields as context cannot be overridden https://github.com/fluxcd/notification-controller/pull/529/files#diff-9c161f071659a6d134b4cfdd27ad66fc88d93b5dfb102d7bb5a768b9e9e483eaR81 as you mentioned they are used by flux already.

@audunsolemdal
Copy link
Author

Looks like this PR described the same use case #340

@makkes
Copy link
Member

makkes commented Aug 1, 2023

Looks like this PR described the same use case #340

I don't think a new field is necessary here as we can rather fetch the needed information from the Alert object's .spec.summary field as suggested here.

@somtochiama
Copy link
Member

maybe we can make the context configurable to include an additional

@makkes maybe we should include it in the description field instead?

@makkes
Copy link
Member

makkes commented Aug 1, 2023

maybe we can make the context configurable to include an additional

@makkes maybe we should include it in the description field instead?

Would be fine from my perspective.

@gdasson
Copy link
Contributor

gdasson commented Dec 29, 2023

@makkes : Please assign this issue to me. I can work on it.

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

No branches or pull requests

4 participants