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.

Sign up for GitHub

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

Jump to bottom

fix: Updated the code to copy the prerenderdata into another variable #43

Merged
merged 1 commit into from
Dec 14, 2023
Merged

fix: Updated the code to copy the prerenderdata into another variable #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/publish-latest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
branches: [master]
types: [completed]
env:
VERSION: v0.0.7
VERSION: v0.0.8
jobs:
deploy:
runs-on: ubuntu-latest
Expand Down
4 changes: 3 additions & 1 deletion controllers/application_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,15 @@ func (r *ApplicationReconciler) renderValues(application *v1.Application) error
}
}
}
preRenderData := GetPreRenderData(application.GetLabels())

for key, value := range values {
buf := new(bytes.Buffer)
tmpl, err := template.New("properties").Option("missingkey=error").Delims(leftDelimiter, rightDelimiter).Parse(value)
if err != nil {
return err
}
if err = tmpl.Execute(buf, GetPreRenderData(application.GetLabels())); err != nil {
if err = tmpl.Execute(buf, preRenderData); err != nil {
return err
}
values[key] = buf.String()
Expand Down
4 changes: 2 additions & 2 deletions controllers/application_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ var pod = &v1.Pod{
}

func LoadTestPrerenderData() {
preRenderData["cluster"] = map[string]string{
clusterData["cluster"] = map[string]string{
"cluster": "some-cluster",
"region": "us-west-2",
"account": "1234",
"segment": "some-segment",
}
preRenderData["egdata"] = map[string]string{
clusterData["egdata"] = map[string]string{
"environment": "test",
}
}
Expand Down
36 changes: 19 additions & 17 deletions controllers/generic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
package controllers

import (
"strings"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/cache"
ctrl "sigs.k8s.io/controller-runtime"
"strings"
)

var preRenderData = make(map[string]map[string]string)

const expediaType = "k8s.expediagroup.com"
var clusterData = make(map[string]map[string]string)

const ReferenceLabel = "overwhelm.expediagroup.com/render-values-source"
const ApplicationKey = "application"
const ExpediaType = "k8s.expediagroup.com"

func LoadPreRenderData() {
func LoadClusterData() {
labelOptions := informers.WithTweakListOptions(func(opts *metav1.ListOptions) {
opts.LabelSelector = ReferenceLabel
})
Expand All @@ -28,35 +26,39 @@ func LoadPreRenderData() {
defer close(stop)
informer.AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(cm interface{}) {
addToPrerenderData(cm.(*v1.ConfigMap))
AddToClusterData(cm.(*v1.ConfigMap))
},
UpdateFunc: func(oldCM interface{}, cm interface{}) {
addToPrerenderData(cm.(*v1.ConfigMap))
AddToClusterData(cm.(*v1.ConfigMap))
},
DeleteFunc: func(cm interface{}) {
delete(preRenderData, cm.(*v1.ConfigMap).Labels[ReferenceLabel])
delete(clusterData, cm.(*v1.ConfigMap).Labels[ReferenceLabel])
},
})
informer.Run(stop)
}

func addToPrerenderData(cm *v1.ConfigMap) {
func AddToClusterData(cm *v1.ConfigMap) {
for k, v := range cm.Data {
if preRenderData[cm.Labels[ReferenceLabel]] == nil {
preRenderData[cm.Labels[ReferenceLabel]] = make(map[string]string)
if clusterData[cm.Labels[ReferenceLabel]] == nil {
clusterData[cm.Labels[ReferenceLabel]] = make(map[string]string)
}
preRenderData[cm.Labels[ReferenceLabel]][k] = v
clusterData[cm.Labels[ReferenceLabel]][k] = v
}
}

func GetPreRenderData(appLabels map[string]string) map[string]map[string]string {
for label, labelValue := range appLabels {
if strings.HasPrefix(label, expediaType) {
func GetPreRenderData(labels map[string]string) map[string]map[string]string {
preRenderData := make(map[string]map[string]string)
for key, value := range clusterData {
preRenderData[key] = value
Copy link
Contributor

Choose a reason for hiding this comment

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

So now clusterData is available to render the values ? Is there any specific
field needed in that ?
I think the field in question was {{application.instance}} which still should come from application labels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea so I renamed prerenderdata to clusterdata as I can see it probably caused confusion. With this change the preRenderData will now have all the cluster level data and the application level data. The change was to copy the original prerenderdata(now named as clusterdata) into another variable since the original clusterdata map was an instance variable and was supposed to hold only cluster level data and its updates from the informer. It shouldnt be updated with application details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hope this made sense?

Copy link
Contributor Author

@sangdammad sangdammad Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I therefore did two things:

  1. renamed prerenderdata to cluster data - to make it clear that it is cluster data
  2. Copied the instance variable cluster data into a local variable and to this local variable, add the application data as well. This local variable will now serve the template rendering for all cluster and application values.
    So, when a new application is updated, since its loading the local variable at runtime, it will always have the application specific details for that release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previously it was adding application details directly to the instance variable which lives permanently as long overwhelm is running. So if new applications are released that have missing labels (like release instance for e.g.,) will not update the instance variable and old data will be rendered into the new release which was one of the issues they were facing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also theres no need to store all the application details in memory which was happening before

}
for label, labelValue := range labels {
if strings.HasPrefix(label, ExpediaType) {
//Initialise the map, even if there is a single label matching criteria
if preRenderData[ApplicationKey] == nil {
preRenderData[ApplicationKey] = make(map[string]string)
}
trimmedLabel := strings.Trim(strings.TrimPrefix(label, expediaType), "/")
trimmedLabel := strings.Trim(strings.TrimPrefix(label, ExpediaType), "/")
preRenderData[ApplicationKey][trimmedLabel] = labelValue
}
}
Expand Down
2 changes: 1 addition & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func main() {
}
setupLog.Info("Loading cluster data")

go controllers.LoadPreRenderData()
go controllers.LoadClusterData()

if err = (&controllers.ApplicationReconciler{
Client: mgr.GetClient(),
Expand Down