Azure / k8s-deploy

GitHub Action for deploying to Kubernetes clusters
MIT License
252 stars 104 forks source link

Add support for TrafficSplit annotations #151

Closed tillig closed 2 years ago

tillig commented 3 years ago

When SMI traffic handling is selected, the TrafficSplit that gets generated (both for blue/green and canary is pretty hard-coded with only weights and service names.

At least with Istio, this will only work for services accessed from within the cluster. To get this working with ingress traffic, the Istio VirtualService needs to have a gateway attached.

The Istio SMI adapter has support for setting this through annotations but there's no way to add annotations to the generated TrafficSplit.

It would be cool to be able to add annotations to the TrafficSplit to support cases like this. It could be a key/value list parameter to the task and just copied into the TrafficSplit when it's generated. If none are present, you get the same thing you get now. (Admittedly, I'm kind of new to GitHub Actions and I'm unclear if it'd need to be a multiline string or if you can have object style parameters like Azure DevOps. Either way is fine.)

Corresponding issue filed for Azure DevOps KubernetesManifest task.

github-actions[bot] commented 3 years ago

This issue is idle because it has been open for 14 days with no activity.

tillig commented 3 years ago

14 days idle seems a little short for feature suggestions. It'd still be good to have this. I haven't moved to GitHub Actions yet but we're targeting that move in my organization for Q1 2022. In the meantime I've forked the baked-in AzDO task and have to own that for the next several months. I can get a PR in for this but it'd be nice to hear from the team if it's interesting before I invest the time.

github-actions[bot] commented 2 years ago

This issue is idle because it has been open for 14 days with no activity.

tillig commented 2 years ago

It would be nice to hear if this is interesting to the team before doing to work to create a pull request.

github-actions[bot] commented 2 years ago

This issue is idle because it has been open for 14 days with no activity.

OliverMKing commented 2 years ago

Hi @tillig. We'd love this feature. Please feel free to create a PR.

tillig commented 2 years ago

Awesome. We're still in the process of getting our GitHub Enterprise system underway. That'll be my test bed for verifying my changes before submitting the PR since I don't really have any other system with everything wired up. However, I do plan on getting a PR in here so I'll keep this open while I work on it. Thanks!

github-actions[bot] commented 2 years ago

This issue is idle because it has been open for 14 days with no activity.

OliverMKing commented 2 years ago

@Vidya2606 is actively working on this.

OliverMKing commented 2 years ago

@tillig do you have any links to documentation talking about using the annotation for Istio SMI adapter? I can see it in the code but I'm struggling to find docs.

tillig commented 2 years ago

Unfortunately, no, I don't have docs. The documentation on the adapter is very light and focuses more on the SMI spec than about using the annotations.

It seems like it doesn't really matter though, right? If there's just the ability to set any arbitrary dictionary of annotations on the TrafficSplit, it doesn't matter what the annotations are, I would think. (I had to fork the SMI controller for an internal project and we have added more annotations than what is in the base product. The annotation support here needs to be pretty open, not something where it gets generated or sanity checked based on other parameters.)

github-actions[bot] commented 2 years ago

This issue is idle because it has been open for 14 days with no activity.

OliverMKing commented 2 years ago

This feature was added to the latest version :). You can use the traffic-split-annotations input. Thanks!