From 658415b9fce26a3dd4f99a86088b563b864dd9b3 Mon Sep 17 00:00:00 2001 From: Ajanth Date: Thu, 21 Oct 2021 21:21:09 +0530 Subject: [PATCH 1/8] Fixing issue 271 Signed-off-by: Ajanth --- operator/controllers/config/config.go | 3 +- operator/controllers/routing_table.go | 57 ++++------------------- operator/main.go | 47 ++++++++++++------- pkg/k8s/config_map.go | 65 --------------------------- 4 files changed, 42 insertions(+), 130 deletions(-) diff --git a/operator/controllers/config/config.go b/operator/controllers/config/config.go index 051b9085..7b4f0a81 100644 --- a/operator/controllers/config/config.go +++ b/operator/controllers/config/config.go @@ -22,7 +22,8 @@ type ExternalScaler struct { } type Base struct { - TargetPendingRequests int32 `envconfig:"TARGET_PENDING_REQUESTS" default:"100"` + TargetPendingRequests int32 `envconfig:"TARGET_PENDING_REQUESTS" default:"100"` + Namespace string `envconfig:"NAMESPACE" required:"true"` } func NewBaseFromEnv() (*Base, error) { diff --git a/operator/controllers/routing_table.go b/operator/controllers/routing_table.go index 7627fb46..defa39f9 100644 --- a/operator/controllers/routing_table.go +++ b/operator/controllers/routing_table.go @@ -7,7 +7,6 @@ import ( "github.com/kedacore/http-add-on/pkg/k8s" "github.com/kedacore/http-add-on/pkg/routing" pkgerrs "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -62,57 +61,17 @@ func updateRoutingMap( ) error { lggr = lggr.WithName("updateRoutingMap") routingConfigMap, err := k8s.GetConfigMap(ctx, cl, namespace, routing.ConfigMapRoutingTableName) - // if there is an error other than not found on the ConfigMap, we should - // fail - if err != nil && !errors.IsNotFound(err) { - lggr.Error( - err, - "other issue fetching the routing table ConfigMap", - "configMapName", - routing.ConfigMapRoutingTableName, - ) + if err != nil { + lggr.Error(err, "Error getting configmap", "configMapName", routing.ConfigMapRoutingTableName) return pkgerrs.Wrap(err, "routing table ConfigMap fetch error") } - // if either the routing table ConfigMap doesn't exist or for some reason it's - // nil in memory, we need to create it - if errors.IsNotFound(err) || routingConfigMap == nil { - lggr.Info( - "routing table ConfigMap didn't exist, creating it", - "configMapName", - routing.ConfigMapRoutingTableName, - ) - routingTableLabels := map[string]string{ - "control-plane": "operator", - "keda.sh/addon": "http-add-on", - "app": "http-add-on", - "name": "http-add-on-routing-table", - } - cm := k8s.NewConfigMap( - namespace, - routing.ConfigMapRoutingTableName, - routingTableLabels, - map[string]string{}, - ) - if err := routing.SaveTableToConfigMap(table, cm); err != nil { - return err - } - if err := k8s.CreateConfigMap( - ctx, - lggr, - cl, - cm, - ); err != nil { - return err - } - } else { - newCM := routingConfigMap.DeepCopy() - if err := routing.SaveTableToConfigMap(table, newCM); err != nil { - return err - } - if _, patchErr := k8s.PatchConfigMap(ctx, lggr, cl, routingConfigMap, newCM); patchErr != nil { - return patchErr - } + newCM := routingConfigMap.DeepCopy() + if err := routing.SaveTableToConfigMap(table, newCM); err != nil { + return err + } + if _, patchErr := k8s.PatchConfigMap(ctx, lggr, cl, routingConfigMap, newCM); patchErr != nil { + return patchErr } return nil diff --git a/operator/main.go b/operator/main.go index aa111952..859f4d65 100644 --- a/operator/main.go +++ b/operator/main.go @@ -33,6 +33,7 @@ import ( httpv1alpha1 "github.com/kedacore/http-add-on/operator/api/v1alpha1" "github.com/kedacore/http-add-on/operator/controllers" "github.com/kedacore/http-add-on/operator/controllers/config" + "github.com/kedacore/http-add-on/pkg/k8s" "github.com/kedacore/http-add-on/pkg/routing" // +kubebuilder:scaffold:imports ) @@ -64,9 +65,29 @@ func main() { "The port on which to run the admin server. This is the port on which RPCs will be accepted to get the routing table", ) flag.Parse() + interceptorCfg, err := config.NewInterceptorFromEnv() + if err != nil { + setupLog.Error(err, "unable to get interceptor configuration") + os.Exit(1) + } + externalScalerCfg, err := config.NewExternalScalerFromEnv() + if err != nil { + setupLog.Error(err, "unable to get external scaler configuration") + os.Exit(1) + } + baseConfig, err := config.NewBaseFromEnv() + if err != nil { + setupLog.Error( + err, + "unable to get base configuration", + ) + os.Exit(1) + } ctrl.SetLogger(zap.New(zap.UseDevMode(true))) + ctx := context.Background() + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, @@ -79,24 +100,21 @@ func main() { os.Exit(1) } - interceptorCfg, err := config.NewInterceptorFromEnv() - if err != nil { - setupLog.Error(err, "unable to get interceptor configuration") - os.Exit(1) - } - externalScalerCfg, err := config.NewExternalScalerFromEnv() - if err != nil { - setupLog.Error(err, "unable to get external scaler configuration") - os.Exit(1) - } - baseConfig, err := config.NewBaseFromEnv() - if err != nil { + cl := mgr.GetClient() + namespace := baseConfig.Namespace + _, getConfigMapError := k8s.GetConfigMap(ctx, cl, namespace, routing.ConfigMapRoutingTableName) + // if there is an error other than not found on the ConfigMap, we should + // fail + if getConfigMapError != nil { setupLog.Error( - err, - "unable to get base configuration", + getConfigMapError, + "Couldn't fetch routing table config map", + "configMapName", + routing.ConfigMapRoutingTableName, ) os.Exit(1) } + routingTable := routing.NewTable() if err := (&controllers.HTTPScaledObjectReconciler{ Client: mgr.GetClient(), @@ -112,7 +130,6 @@ func main() { } // +kubebuilder:scaffold:builder - ctx := context.Background() errGrp, _ := errgroup.WithContext(ctx) // start the control loop diff --git a/pkg/k8s/config_map.go b/pkg/k8s/config_map.go index a4a46dff..abd32d05 100644 --- a/pkg/k8s/config_map.go +++ b/pkg/k8s/config_map.go @@ -38,71 +38,6 @@ type ConfigMapGetterWatcher interface { ConfigMapWatcher } -// newConfigMap creates a new configMap structure -func NewConfigMap( - namespace string, - name string, - labels map[string]string, - data map[string]string, -) *corev1.ConfigMap { - - configMap := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - Kind: "ConfigMap", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: labels, - }, - Data: data, - } - - return configMap -} - -// CreateConfigMap sends a request to Kubernetes using the client cl -// to create configMap. Returns a non-nil error if anything failed with the creation, -// including if the config map already existed. -func CreateConfigMap( - ctx context.Context, - logger logr.Logger, - cl client.Writer, - configMap *corev1.ConfigMap, -) error { - logger = logger.WithName("pkg.k8s.CreateConfigMap") - if err := cl.Create(ctx, configMap); err != nil { - logger.Error( - err, - "failed to create ConfigMap", - "configMap", - *configMap, - ) - return err - } - return nil -} - -func DeleteConfigMap( - ctx context.Context, - cl client.Writer, - configMap *corev1.ConfigMap, - logger logr.Logger, -) error { - logger = logger.WithName("pkg.k8s.DeleteConfigMap") - err := cl.Delete(ctx, configMap) - if err != nil { - logger.Error( - err, - "failed to delete configmap", - "configMap", - *configMap, - ) - return err - } - return nil -} - func PatchConfigMap( ctx context.Context, logger logr.Logger, From c2fe457f230ae7061eca1a376c59ef190f2eaa5c Mon Sep 17 00:00:00 2001 From: Tom Kerkhove Date: Thu, 14 Oct 2021 19:30:59 +0200 Subject: [PATCH 2/8] chore: Provide configuration for automatically closing inactive issues (#298) * chore: Provide configuration for automatically closing inactive issues Signed-off-by: GitHub * Align with core Signed-off-by: GitHub * feature instead of feature-request Signed-off-by: GitHub Signed-off-by: Ajanth --- .github/stale.yml | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .github/stale.yml diff --git a/.github/stale.yml b/.github/stale.yml new file mode 100644 index 00000000..979cb1a4 --- /dev/null +++ b/.github/stale.yml @@ -0,0 +1,43 @@ +# Number of days of inactivity before an issue becomes stale +daysUntilStale: 60 + +# Number of days of inactivity before a stale issue is closed +daysUntilClose: 7 + +# Issues with these labels will never be considered stale +exemptLabels: + - cant-touch-this + - feature + - security + +# Label to use when marking an issue as stale +staleLabel: stale + +# Set to true to ignore issues in a project (defaults to false) +exemptProjects: false + +# Set to true to ignore issues in a milestone (defaults to false) +exemptMilestones: false + +# Set to true to ignore issues with an assignee (defaults to false) +exemptAssignees: false + +# Comment to post when marking an issue as stale. Set to `false` to disable +markComment: > + This issue has been automatically marked as stale because it has not had + recent activity. It will be closed in 7 days if no further activity occurs. Thank you + for your contributions. + +# Comment to post when removing the stale label. +# unmarkComment: > +# Your comment here. + +# Comment to post when closing a stale Issue or Pull Request. +closeComment: > + This issue has been automatically closed due to inactivity. + +# Limit the number of actions per hour, from 1-30. Default is 30 +limitPerRun: 30 + +# Limit to only `issues` or `pulls` +only: issues \ No newline at end of file From 2865803cc950894bff275aa390e6a34549e2c6c7 Mon Sep 17 00:00:00 2001 From: Ajanth <50458502+ajanth97@users.noreply.github.com> Date: Thu, 21 Oct 2021 21:56:25 +0530 Subject: [PATCH 3/8] Update operator/controllers/routing_table.go Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth --- operator/controllers/routing_table.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/operator/controllers/routing_table.go b/operator/controllers/routing_table.go index defa39f9..eea8a856 100644 --- a/operator/controllers/routing_table.go +++ b/operator/controllers/routing_table.go @@ -70,8 +70,9 @@ func updateRoutingMap( if err := routing.SaveTableToConfigMap(table, newCM); err != nil { return err } - if _, patchErr := k8s.PatchConfigMap(ctx, lggr, cl, routingConfigMap, newCM); patchErr != nil { - return patchErr + if _, err := k8s.PatchConfigMap(ctx, lggr, cl, routingConfigMap, newCM); err != nil { + lggr.Error(err, "couldn't save new routing table ConfigMap to Kubernetes", "configMap", routing.ConfigMapRoutingTableName) + return pkgerrs.Wrap(err, "saving routing table ConfigMap to Kubernetes") } return nil From 8bb327a012a48786d1a80a2b9bac16126f76a65c Mon Sep 17 00:00:00 2001 From: Ajanth <50458502+ajanth97@users.noreply.github.com> Date: Thu, 21 Oct 2021 21:57:42 +0530 Subject: [PATCH 4/8] Update operator/main.go Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth --- operator/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/main.go b/operator/main.go index 859f4d65..11317fc5 100644 --- a/operator/main.go +++ b/operator/main.go @@ -117,7 +117,7 @@ func main() { routingTable := routing.NewTable() if err := (&controllers.HTTPScaledObjectReconciler{ - Client: mgr.GetClient(), + Client: cl, Log: ctrl.Log.WithName("controllers").WithName("HTTPScaledObject"), Scheme: mgr.GetScheme(), InterceptorConfig: *interceptorCfg, From a7e506caec72c5183cb7ebd3d940b87c436db752 Mon Sep 17 00:00:00 2001 From: Ajanth <50458502+ajanth97@users.noreply.github.com> Date: Thu, 21 Oct 2021 21:58:33 +0530 Subject: [PATCH 5/8] Update operator/main.go Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth --- operator/main.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/operator/main.go b/operator/main.go index 11317fc5..61307fc7 100644 --- a/operator/main.go +++ b/operator/main.go @@ -102,10 +102,8 @@ func main() { cl := mgr.GetClient() namespace := baseConfig.Namespace - _, getConfigMapError := k8s.GetConfigMap(ctx, cl, namespace, routing.ConfigMapRoutingTableName) - // if there is an error other than not found on the ConfigMap, we should - // fail - if getConfigMapError != nil { + // crash if the routing table ConfigMap couldn't be found + if _, err := k8s.GetConfigMap(ctx, cl, namespace, routing.ConfigMapRoutingTableName); err != nil { setupLog.Error( getConfigMapError, "Couldn't fetch routing table config map", From ab71afef938d283f1b5e92944981a17275d6f218 Mon Sep 17 00:00:00 2001 From: Ajanth <50458502+ajanth97@users.noreply.github.com> Date: Thu, 21 Oct 2021 21:59:22 +0530 Subject: [PATCH 6/8] Update operator/main.go Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth --- operator/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/main.go b/operator/main.go index 61307fc7..f41efd66 100644 --- a/operator/main.go +++ b/operator/main.go @@ -105,7 +105,7 @@ func main() { // crash if the routing table ConfigMap couldn't be found if _, err := k8s.GetConfigMap(ctx, cl, namespace, routing.ConfigMapRoutingTableName); err != nil { setupLog.Error( - getConfigMapError, + err, "Couldn't fetch routing table config map", "configMapName", routing.ConfigMapRoutingTableName, From 01c968380b3c4f96a2943225b4dac6e6b00f627d Mon Sep 17 00:00:00 2001 From: Ajanth <50458502+ajanth97@users.noreply.github.com> Date: Fri, 22 Oct 2021 12:59:24 +0530 Subject: [PATCH 7/8] Update operator/controllers/routing_table.go Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth --- operator/controllers/routing_table.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/operator/controllers/routing_table.go b/operator/controllers/routing_table.go index eea8a856..caf24b0a 100644 --- a/operator/controllers/routing_table.go +++ b/operator/controllers/routing_table.go @@ -68,7 +68,8 @@ func updateRoutingMap( newCM := routingConfigMap.DeepCopy() if err := routing.SaveTableToConfigMap(table, newCM); err != nil { - return err + lggr.Error(err, "couldn't save new routing table to ConfigMap", "configMap", routing.ConfigMapRoutingTableName) + return pkgerrs.Wrap(err, "ConfigMap save error") } if _, err := k8s.PatchConfigMap(ctx, lggr, cl, routingConfigMap, newCM); err != nil { lggr.Error(err, "couldn't save new routing table ConfigMap to Kubernetes", "configMap", routing.ConfigMapRoutingTableName) From 9e1f9b4ee4d0f88bded71d5c7f813a1f32ca9296 Mon Sep 17 00:00:00 2001 From: Ajanth Date: Fri, 5 Nov 2021 21:21:35 +0530 Subject: [PATCH 8/8] Fixing test and an indentation Signed-off-by: Ajanth --- operator/controllers/routing_table.go | 5 ++--- operator/controllers/routing_table_test.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/operator/controllers/routing_table.go b/operator/controllers/routing_table.go index caf24b0a..8ca26291 100644 --- a/operator/controllers/routing_table.go +++ b/operator/controllers/routing_table.go @@ -65,14 +65,13 @@ func updateRoutingMap( lggr.Error(err, "Error getting configmap", "configMapName", routing.ConfigMapRoutingTableName) return pkgerrs.Wrap(err, "routing table ConfigMap fetch error") } - newCM := routingConfigMap.DeepCopy() if err := routing.SaveTableToConfigMap(table, newCM); err != nil { - lggr.Error(err, "couldn't save new routing table to ConfigMap", "configMap", routing.ConfigMapRoutingTableName) + lggr.Error(err, "couldn't save new routing table to ConfigMap", "configMap", routing.ConfigMapRoutingTableName) return pkgerrs.Wrap(err, "ConfigMap save error") } if _, err := k8s.PatchConfigMap(ctx, lggr, cl, routingConfigMap, newCM); err != nil { - lggr.Error(err, "couldn't save new routing table ConfigMap to Kubernetes", "configMap", routing.ConfigMapRoutingTableName) + lggr.Error(err, "couldn't save new routing table ConfigMap to Kubernetes", "configMap", routing.ConfigMapRoutingTableName) return pkgerrs.Wrap(err, "saving routing table ConfigMap to Kubernetes") } diff --git a/operator/controllers/routing_table_test.go b/operator/controllers/routing_table_test.go index 7c723416..b9c800eb 100644 --- a/operator/controllers/routing_table_test.go +++ b/operator/controllers/routing_table_test.go @@ -7,6 +7,8 @@ import ( "github.com/go-logr/logr" "github.com/kedacore/http-add-on/pkg/routing" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -20,7 +22,20 @@ func TestRoutingTable(t *testing.T) { ) r := require.New(t) ctx := context.Background() + configMap := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: "keda-http-routing-table", + }, + Data: map[string]string{ + //An error would occur if this map is left empty. + //It would be nil probably because of the deep copy. + "hello": "Hello", + }, + } cl := fake.NewClientBuilder().Build() + r.NoError(cl.Create(ctx, &configMap)) + target := routing.Target{ Service: svcName, Port: 8080,