You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "valdar (via GitHub)" <gi...@apache.org> on 2024/01/29 17:40:00 UTC
[PR] fix #4948 part of #3397 [camel-k]
valdar opened a new pull request, #5119:
URL: https://github.com/apache/camel-k/pull/5119
<!-- Description -->
Fix for #4948 that is part of #3397
A `platformcontroller` command is added as subcommand of `kamel`; it runs an operator like the `operator` subcomand does, but platformcontroller would just handle `IntegrationPlatform` CRs. All the other CRDs are handled by operator as before.
These installation methods have been amended accordingly:
- `install` command
- olm bundle
- helm charts
e2e tests has been ran locally as well.
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505458111
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ LoggerSetup() (logutil.Logger, error, string)
+ PrintVersion()
+ GetWatchNamespace() (string, error)
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string)
+ CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string)
+ GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string)
+ SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string)
+ GetManagerOptions(bootstrapClient client.Client, watchNamespace string) (cache.Options, error, string)
+ CreateManager(healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options, ctx context.Context) (manager.Manager, client.Client, error, string)
+ ControllerPreStartResourcesInit(initCtx context.Context, bootstrapClient client.Client, watchNamespace string, controllerNamespace string, ctx context.Context, ctrlClient client.Client, mgr manager.Manager) (error, string)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (error, string) {
+ log, err, errMessage := c.ControllerManager.LoggerSetup()
+ if err != nil {
+ log.Error(err, errMessage)
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ log.Debug("*** DEBUG level messages will be logged ***")
+
+ watchNamespace, err := c.ControllerManager.GetWatchNamespace()
+ if err != nil {
+ return err, "failed to get watch namespace"
+ }
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return err, "cannot get client config"
+ }
+
+ bootstrapClient, err, errMessage := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return err, errMessage
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, err, errMessage := c.ControllerManager.CreateEventBroadcaster(bootstrapClient, watchNamespace, ctx)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return err, errMessage
+ }
+
+ controllerNamespace, leaderElection, err, errMessage := c.ControllerManager.GetControllerNamespaceAndLeaderElection(watchNamespace, bootstrapClient, leaderElection, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+ if !leaderElection {
+ log.Info("Leader election is disabled!")
+ }
+
+ err, errMessage = c.ControllerManager.SetOperatorImage(bootstrapClient, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+
+ options, err, errMessage := c.ControllerManager.GetManagerOptions(bootstrapClient, watchNamespace)
+ if err != nil {
+ return err, errMessage
+ }
+
+ mgr, ctrlClient, err, errMessage := c.ControllerManager.CreateManager(healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options, ctx)
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ err, errMessage = c.ControllerManager.ControllerPreStartResourcesInit(initCtx, bootstrapClient, watchNamespace, controllerNamespace, ctx, ctrlClient, mgr)
+ if err != nil {
+ return err, errMessage
+ }
+
+ log.Info("Starting the manager")
+ return mgr.Start(ctx), "manager exited non-zero"
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespaceEnvVar string
+ ControllerNamespace string
+ ControllerPodName string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) LoggerSetup() (logutil.Logger, error, string) {
+ // The logger instantiated here can be changed to any logger
+ // implementing the logr.Logger interface. This logger will
+ // be propagated through the whole operator, generating
+ // uniform and structured logs.
+
+ // The constants specified here are zap specific
+ var logLevel zapcore.Level
+ logLevelVal, ok := os.LookupEnv("LOG_LEVEL")
+ if ok {
+ switch strings.ToLower(logLevelVal) {
+ case "error":
+ logLevel = zapcore.ErrorLevel
+ case "info":
+ logLevel = zapcore.InfoLevel
+ case "debug":
+ logLevel = zapcore.DebugLevel
+ default:
+ customLevel, err := strconv.Atoi(strings.ToLower(logLevelVal))
+ if err != nil {
+ return bm.Log, err, "Invalid log-level"
+ }
+ // Need to multiply by -1 to turn logr expected level into zap level
+ logLevel = zapcore.Level(int8(customLevel) * -1)
+ }
+ } else {
+ logLevel = zapcore.InfoLevel
+ }
+
+ // Use and set atomic level that all following log events are compared with
+ // in order to evaluate if a given log level on the event is enabled.
+ logf.SetLogger(zapctrl.New(func(o *zapctrl.Options) {
+ o.Development = false
+ o.Level = zap.NewAtomicLevelAt(logLevel)
+ }))
+
+ klog.SetLogger(bm.Log.AsLogger())
+
+ _, err := maxprocs.Set(maxprocs.Logger(func(f string, a ...interface{}) { bm.Log.Info(fmt.Sprintf(f, a)) }))
+
+ return bm.Log, err, "failed to set GOMAXPROCS from cgroups"
+}
+
+// getWatchNamespace returns the Namespace the operator should be watching for changes.
+func (bm BaseManager) GetWatchNamespace() (string, error) {
+ ns, found := os.LookupEnv(bm.WatchNamespaceEnvVar)
Review Comment:
looking at how the `WatchNamespaceEnvVar` is implemented, this can probably be just a function in an utility/platform package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505496052
##########
pkg/cmd/operator/operator.go:
##########
@@ -70,122 +51,48 @@ import (
var log = logutil.Log.WithName("cmd")
Review Comment:
may be better to use a better log name
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2058785527
:warning: Unit test coverage report - coverage decreased from 38% to 37.9% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1504950108
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
Review Comment:
this and the default implementation should probably be in a dedicated package, otherwise the operator sub command has to import the manager sub command
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "valdar (via GitHub)" <gi...@apache.org>.
valdar commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1511237712
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,218 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "strconv"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ PrintVersion()
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error)
+ CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error)
+ GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error)
+ GetManagerOptions(bootstrapClient client.Client) (cache.Options, string, error)
+ CreateManager(ctx context.Context, healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options) (manager.Manager, client.Client, string, error)
+ ControllerPreStartResourcesInit(ctx context.Context, initCtx context.Context, bootstrapClient client.Client, controllerNamespace string, ctrlClient client.Client, mgr manager.Manager) (string, error)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+ Log logutil.Logger
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (string, error) {
+ errMessage, err := setMaxprocs(c.Log)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ c.Log.Debug("*** DEBUG level messages will be logged ***")
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return "cannot get client config", err
+ }
+ bootstrapClient, errMessage, err := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return errMessage, err
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, errMessage, err := c.ControllerManager.CreateEventBroadcaster(ctx, bootstrapClient)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return errMessage, err
+ }
+
+ controllerNamespace, leaderElection, errMessage, err := c.ControllerManager.GetControllerNamespaceAndLeaderElection(ctx, bootstrapClient, leaderElection)
+ if err != nil {
+ return errMessage, err
+ }
+ if !leaderElection {
+ c.Log.Info("Leader election is disabled!")
+ }
+
+ errMessage, err = setOperatorImage(ctx, bootstrapClient, controllerNamespace)
+ if err != nil {
+ return errMessage, err
+ }
+
+ options, errMessage, err := c.ControllerManager.GetManagerOptions(bootstrapClient)
+ if err != nil {
+ return errMessage, err
+ }
+
+ mgr, ctrlClient, errMessage, err := c.ControllerManager.CreateManager(ctx, healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options)
+ if err != nil {
+ return errMessage, err
+ }
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ errMessage, err = c.ControllerManager.ControllerPreStartResourcesInit(ctx, initCtx, bootstrapClient, controllerNamespace, ctrlClient, mgr)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.Log.Info("Starting the manager")
+ return "manager exited non-zero", mgr.Start(ctx)
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespace string
+ ControllerNamespace string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error) {
+ // Increase maximum burst that is used by client-side throttling,
+ // to prevent the requests made to apply the bundled Kamelets
+ // from being throttled.
+ cfg.QPS = 20
+ cfg.Burst = 200
+ bootstrapClient, err := client.NewClientWithConfig(false, cfg)
+ if err != nil {
+ return nil, "cannot initialize client", err
+ }
+
+ return bootstrapClient, "", nil
+}
+
+func (bm BaseManager) CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error) {
+ // We do not rely on the event broadcaster managed by controller runtime,
+ // so that we can check the operator has been granted permission to create
+ // Events. This is required for the operator to be installable by standard
+ // admin users, that are not granted create permission on Events by default.
+ broadcaster := record.NewBroadcaster()
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, corev1.GroupName, "events", bm.WatchNamespace, "", "create"); err != nil || !ok {
Review Comment:
I think you addressed it by opening this https://github.com/apache/camel-k/issues/5210
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2061230296
:warning: Unit test coverage report - coverage decreased from 38% to 37.9% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2006744346
Also, several tests have failed and they don't seem to be the usual flaky ones. I've restarted them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1504961262
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ LoggerSetup() (logutil.Logger, error, string)
+ PrintVersion()
+ GetWatchNamespace() (string, error)
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string)
+ CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string)
+ GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string)
+ SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string)
+ GetManagerOptions(bootstrapClient client.Client, watchNamespace string) (cache.Options, error, string)
+ CreateManager(healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options, ctx context.Context) (manager.Manager, client.Client, error, string)
+ ControllerPreStartResourcesInit(initCtx context.Context, bootstrapClient client.Client, watchNamespace string, controllerNamespace string, ctx context.Context, ctrlClient client.Client, mgr manager.Manager) (error, string)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (error, string) {
+ log, err, errMessage := c.ControllerManager.LoggerSetup()
+ if err != nil {
+ log.Error(err, errMessage)
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ log.Debug("*** DEBUG level messages will be logged ***")
+
+ watchNamespace, err := c.ControllerManager.GetWatchNamespace()
+ if err != nil {
+ return err, "failed to get watch namespace"
+ }
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return err, "cannot get client config"
+ }
+
+ bootstrapClient, err, errMessage := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return err, errMessage
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, err, errMessage := c.ControllerManager.CreateEventBroadcaster(bootstrapClient, watchNamespace, ctx)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return err, errMessage
+ }
+
+ controllerNamespace, leaderElection, err, errMessage := c.ControllerManager.GetControllerNamespaceAndLeaderElection(watchNamespace, bootstrapClient, leaderElection, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+ if !leaderElection {
+ log.Info("Leader election is disabled!")
+ }
+
+ err, errMessage = c.ControllerManager.SetOperatorImage(bootstrapClient, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+
+ options, err, errMessage := c.ControllerManager.GetManagerOptions(bootstrapClient, watchNamespace)
+ if err != nil {
+ return err, errMessage
+ }
+
+ mgr, ctrlClient, err, errMessage := c.ControllerManager.CreateManager(healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options, ctx)
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ err, errMessage = c.ControllerManager.ControllerPreStartResourcesInit(initCtx, bootstrapClient, watchNamespace, controllerNamespace, ctx, ctrlClient, mgr)
+ if err != nil {
+ return err, errMessage
+ }
+
+ log.Info("Starting the manager")
+ return mgr.Start(ctx), "manager exited non-zero"
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespaceEnvVar string
+ ControllerNamespace string
+ ControllerPodName string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) LoggerSetup() (logutil.Logger, error, string) {
+ // The logger instantiated here can be changed to any logger
+ // implementing the logr.Logger interface. This logger will
+ // be propagated through the whole operator, generating
+ // uniform and structured logs.
+
+ // The constants specified here are zap specific
+ var logLevel zapcore.Level
+ logLevelVal, ok := os.LookupEnv("LOG_LEVEL")
+ if ok {
+ switch strings.ToLower(logLevelVal) {
+ case "error":
+ logLevel = zapcore.ErrorLevel
+ case "info":
+ logLevel = zapcore.InfoLevel
+ case "debug":
+ logLevel = zapcore.DebugLevel
+ default:
+ customLevel, err := strconv.Atoi(strings.ToLower(logLevelVal))
+ if err != nil {
+ return bm.Log, err, "Invalid log-level"
+ }
+ // Need to multiply by -1 to turn logr expected level into zap level
+ logLevel = zapcore.Level(int8(customLevel) * -1)
+ }
+ } else {
+ logLevel = zapcore.InfoLevel
+ }
+
+ // Use and set atomic level that all following log events are compared with
+ // in order to evaluate if a given log level on the event is enabled.
+ logf.SetLogger(zapctrl.New(func(o *zapctrl.Options) {
+ o.Development = false
+ o.Level = zap.NewAtomicLevelAt(logLevel)
+ }))
+
+ klog.SetLogger(bm.Log.AsLogger())
+
+ _, err := maxprocs.Set(maxprocs.Logger(func(f string, a ...interface{}) { bm.Log.Info(fmt.Sprintf(f, a)) }))
Review Comment:
this probably does not belong to the log setup function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "valdar (via GitHub)" <gi...@apache.org>.
valdar commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1511254321
##########
pkg/cmd/platformcontroller/platformcontroller.go:
##########
@@ -0,0 +1,272 @@
+/*
Review Comment:
I have abstracted as much code as I could in the cmd/controller/manager.go interface and implementations. Hope it is a suitable solution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1976276298
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "valdar (via GitHub)" <gi...@apache.org>.
valdar commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1511244894
##########
e2e/support/test_support.go:
##########
@@ -2245,7 +2253,7 @@ func OperatorPod(ns string) func() *corev1.Pod {
if err := TestClient().List(TestContext, &lst,
ctrl.InNamespace(ns),
ctrl.MatchingLabels{
- "camel.apache.org/component": "operator",
+ "camel.apache.org/component": componentLabelValue,
Review Comment:
Opened a separate issue for that https://github.com/apache/camel-k/issues/5212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2009258746
:warning: Unit test coverage report - coverage decreased from 37.3% to 37.2% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2007195419
> @lburgazzoli the problem is not the failure of the checks per se. As we've done in other PR, everything that may affect the Quarkus native has to be validated manually in order to make sure we don't introduce any regression. Please, have a look at this PR for instance: [#5234 (comment)](https://github.com/apache/camel-k/pull/5234#issuecomment-1985493127)
>
> If the folks have any problem running the native tests, you or I can run them and publish here the results, although, as suggested to some other contributor, it would be good to be able to do it on their own as it's should be a normal practice to do when doing some development. Running locally it is as easy as running any other integration test [1], it is only taking a bit of time more.
>
I'm not complaining about having to run them locally, I'm just noticing the OSX build is almost always failing so I wonder what is the value of such test. Now I can run the test locally on my linux box and reporting them as ok but, what I've testes is not what has failed so people may spend time waiting and trying to find out why things are not working on OSX whereas it is an environment issue.
> Beside, as mentioned in [2] my personal feeling is that these kind of changes would require some documentation and some rationale explanation as the users that will move from 2.2 to 2.3 will have the surprise of a multi deployment that may require them to adjust monitoring and other operational aspects. To me this is a breaking change, but if you want to proceed, at least it has to be clearly explained to the final users.
>
I agree that we need some docs but since this PR is part of a larger issue and the rationale was briefly explained in https://github.com/apache/camel-k/issues/3397#issuecomment-1735525398, then I wonder if the doc could be something we do as a separate PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "valdar (via GitHub)" <gi...@apache.org>.
valdar commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1925205616
What about these tests:
❌ TestMetrics (3m0.27s)
❌ TestMetrics/reconciliation_duration_metric (10ms)
they seems flaky to me, first run they were passing, they are passing locally, but now failing in CI :/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505456692
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ LoggerSetup() (logutil.Logger, error, string)
+ PrintVersion()
+ GetWatchNamespace() (string, error)
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string)
+ CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string)
+ GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string)
+ SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string)
+ GetManagerOptions(bootstrapClient client.Client, watchNamespace string) (cache.Options, error, string)
+ CreateManager(healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options, ctx context.Context) (manager.Manager, client.Client, error, string)
+ ControllerPreStartResourcesInit(initCtx context.Context, bootstrapClient client.Client, watchNamespace string, controllerNamespace string, ctx context.Context, ctrlClient client.Client, mgr manager.Manager) (error, string)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (error, string) {
+ log, err, errMessage := c.ControllerManager.LoggerSetup()
+ if err != nil {
+ log.Error(err, errMessage)
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ log.Debug("*** DEBUG level messages will be logged ***")
+
+ watchNamespace, err := c.ControllerManager.GetWatchNamespace()
+ if err != nil {
+ return err, "failed to get watch namespace"
+ }
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return err, "cannot get client config"
+ }
+
+ bootstrapClient, err, errMessage := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return err, errMessage
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, err, errMessage := c.ControllerManager.CreateEventBroadcaster(bootstrapClient, watchNamespace, ctx)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return err, errMessage
+ }
+
+ controllerNamespace, leaderElection, err, errMessage := c.ControllerManager.GetControllerNamespaceAndLeaderElection(watchNamespace, bootstrapClient, leaderElection, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+ if !leaderElection {
+ log.Info("Leader election is disabled!")
+ }
+
+ err, errMessage = c.ControllerManager.SetOperatorImage(bootstrapClient, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+
+ options, err, errMessage := c.ControllerManager.GetManagerOptions(bootstrapClient, watchNamespace)
+ if err != nil {
+ return err, errMessage
+ }
+
+ mgr, ctrlClient, err, errMessage := c.ControllerManager.CreateManager(healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options, ctx)
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ err, errMessage = c.ControllerManager.ControllerPreStartResourcesInit(initCtx, bootstrapClient, watchNamespace, controllerNamespace, ctx, ctrlClient, mgr)
+ if err != nil {
+ return err, errMessage
+ }
+
+ log.Info("Starting the manager")
+ return mgr.Start(ctx), "manager exited non-zero"
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespaceEnvVar string
+ ControllerNamespace string
+ ControllerPodName string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) LoggerSetup() (logutil.Logger, error, string) {
+ // The logger instantiated here can be changed to any logger
+ // implementing the logr.Logger interface. This logger will
+ // be propagated through the whole operator, generating
+ // uniform and structured logs.
+
+ // The constants specified here are zap specific
+ var logLevel zapcore.Level
+ logLevelVal, ok := os.LookupEnv("LOG_LEVEL")
+ if ok {
+ switch strings.ToLower(logLevelVal) {
+ case "error":
+ logLevel = zapcore.ErrorLevel
+ case "info":
+ logLevel = zapcore.InfoLevel
+ case "debug":
+ logLevel = zapcore.DebugLevel
+ default:
+ customLevel, err := strconv.Atoi(strings.ToLower(logLevelVal))
+ if err != nil {
+ return bm.Log, err, "Invalid log-level"
+ }
+ // Need to multiply by -1 to turn logr expected level into zap level
+ logLevel = zapcore.Level(int8(customLevel) * -1)
+ }
+ } else {
+ logLevel = zapcore.InfoLevel
+ }
+
+ // Use and set atomic level that all following log events are compared with
+ // in order to evaluate if a given log level on the event is enabled.
+ logf.SetLogger(zapctrl.New(func(o *zapctrl.Options) {
+ o.Development = false
+ o.Level = zap.NewAtomicLevelAt(logLevel)
+ }))
+
+ klog.SetLogger(bm.Log.AsLogger())
+
+ _, err := maxprocs.Set(maxprocs.Logger(func(f string, a ...interface{}) { bm.Log.Info(fmt.Sprintf(f, a)) }))
+
+ return bm.Log, err, "failed to set GOMAXPROCS from cgroups"
+}
+
+// getWatchNamespace returns the Namespace the operator should be watching for changes.
+func (bm BaseManager) GetWatchNamespace() (string, error) {
+ ns, found := os.LookupEnv(bm.WatchNamespaceEnvVar)
+ if !found {
+ return "", fmt.Errorf("%s must be set", bm.WatchNamespaceEnvVar)
+ }
+ return ns, nil
+}
+
+func (bm BaseManager) CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string) {
+ // Increase maximum burst that is used by client-side throttling,
+ // to prevent the requests made to apply the bundled Kamelets
+ // from being throttled.
+ cfg.QPS = 20
+ cfg.Burst = 200
+ bootstrapClient, err := client.NewClientWithConfig(false, cfg)
+ if err != nil {
+ return nil, err, "cannot initialize client"
+ }
+
+ return bootstrapClient, nil, ""
+}
+
+func (bm BaseManager) CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string) {
+ // We do not rely on the event broadcaster managed by controller runtime,
+ // so that we can check the operator has been granted permission to create
+ // Events. This is required for the operator to be installable by standard
+ // admin users, that are not granted create permission on Events by default.
+ broadcaster := record.NewBroadcaster()
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, corev1.GroupName, "events", watchNamespace, "", "create"); err != nil || !ok {
+ // Do not sink Events to the server as they'll be rejected
+ broadcaster = event.NewSinkLessBroadcaster(broadcaster)
+ return broadcaster, err, "cannot check permissions for creating Events"
+ bm.Log.Info("Event broadcasting is disabled because of missing permissions to create Events")
+ }
+
+ return broadcaster, nil, ""
+}
+
+func (bm BaseManager) GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string) {
+ operatorNamespace := platform.GetOperatorNamespace()
+ if operatorNamespace == "" {
+ // Fallback to using the watch namespace when the operator is not in-cluster.
+ // It does not support local (off-cluster) operator watching resources globally,
+ // in which case it's not possible to determine a namespace.
+ operatorNamespace = watchNamespace
+ if operatorNamespace == "" {
+ leaderElection = false
+ bm.Log.Info("unable to determine namespace for leader election")
+ }
+ }
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, coordination.GroupName, "leases", operatorNamespace, "", "create"); err != nil || !ok {
+ leaderElection = false
+ if err != nil {
+ return operatorNamespace, leaderElection, err, "cannot check permissions for creating Leases"
+ }
+ bm.Log.Info("The operator is not granted permissions to create Leases")
+ }
+
+ return operatorNamespace, leaderElection, nil, ""
+}
+
+// SetOperatorImage set the operator container image if it runs in-container
+func (bm BaseManager) SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string) {
Review Comment:
it seems this function does not really belong to the `Manager` interface as in fact it sets the image elsewhere. It may be moved to a simple function to an utility/platform package so it may simplify the `Manager` interface.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1470871958
##########
config/manager/platformcontroller-deployment.yaml:
##########
@@ -0,0 +1,89 @@
+# ---------------------------------------------------------------------------
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+# ---------------------------------------------------------------------------
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: camel-k-platformcontroller
+ labels:
+ app: "camel-k"
+ camel.apache.org/component: operator
+ name: camel-k-platformcontroller
+ app.kubernetes.io/component: operator
+ app.kubernetes.io/name: camel-k
+ app.kubernetes.io/version: "2.2.0-SNAPSHOT"
Review Comment:
```suggestion
app.kubernetes.io/version: "2.3.0-SNAPSHOT"
```
##########
config/manager/platformcontroller-deployment.yaml:
##########
@@ -0,0 +1,89 @@
+# ---------------------------------------------------------------------------
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+# ---------------------------------------------------------------------------
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: camel-k-platformcontroller
+ labels:
+ app: "camel-k"
+ camel.apache.org/component: operator
+ name: camel-k-platformcontroller
+ app.kubernetes.io/component: operator
+ app.kubernetes.io/name: camel-k
+ app.kubernetes.io/version: "2.2.0-SNAPSHOT"
+spec:
+ replicas: 1
+ strategy:
+ type: Recreate
+ selector:
+ matchLabels:
+ name: camel-k-platformcontroller
+ template:
+ metadata:
+ labels:
+ name: camel-k-platformcontroller
+ camel.apache.org/component: operator
+ app: "camel-k"
+ app.kubernetes.io/component: operator
+ app.kubernetes.io/name: camel-k
+ app.kubernetes.io/version: "2.2.0-SNAPSHOT"
Review Comment:
```suggestion
app.kubernetes.io/version: "2.3.0-SNAPSHOT"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1947370184
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.7% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505514030
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ LoggerSetup() (logutil.Logger, error, string)
+ PrintVersion()
+ GetWatchNamespace() (string, error)
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string)
+ CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string)
+ GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string)
+ SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string)
+ GetManagerOptions(bootstrapClient client.Client, watchNamespace string) (cache.Options, error, string)
+ CreateManager(healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options, ctx context.Context) (manager.Manager, client.Client, error, string)
+ ControllerPreStartResourcesInit(initCtx context.Context, bootstrapClient client.Client, watchNamespace string, controllerNamespace string, ctx context.Context, ctrlClient client.Client, mgr manager.Manager) (error, string)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (error, string) {
+ log, err, errMessage := c.ControllerManager.LoggerSetup()
+ if err != nil {
+ log.Error(err, errMessage)
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ log.Debug("*** DEBUG level messages will be logged ***")
+
+ watchNamespace, err := c.ControllerManager.GetWatchNamespace()
+ if err != nil {
+ return err, "failed to get watch namespace"
+ }
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return err, "cannot get client config"
+ }
+
+ bootstrapClient, err, errMessage := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return err, errMessage
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, err, errMessage := c.ControllerManager.CreateEventBroadcaster(bootstrapClient, watchNamespace, ctx)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return err, errMessage
+ }
+
+ controllerNamespace, leaderElection, err, errMessage := c.ControllerManager.GetControllerNamespaceAndLeaderElection(watchNamespace, bootstrapClient, leaderElection, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+ if !leaderElection {
+ log.Info("Leader election is disabled!")
+ }
+
+ err, errMessage = c.ControllerManager.SetOperatorImage(bootstrapClient, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+
+ options, err, errMessage := c.ControllerManager.GetManagerOptions(bootstrapClient, watchNamespace)
+ if err != nil {
+ return err, errMessage
+ }
+
+ mgr, ctrlClient, err, errMessage := c.ControllerManager.CreateManager(healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options, ctx)
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ err, errMessage = c.ControllerManager.ControllerPreStartResourcesInit(initCtx, bootstrapClient, watchNamespace, controllerNamespace, ctx, ctrlClient, mgr)
+ if err != nil {
+ return err, errMessage
+ }
+
+ log.Info("Starting the manager")
+ return mgr.Start(ctx), "manager exited non-zero"
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespaceEnvVar string
+ ControllerNamespace string
+ ControllerPodName string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) LoggerSetup() (logutil.Logger, error, string) {
+ // The logger instantiated here can be changed to any logger
+ // implementing the logr.Logger interface. This logger will
+ // be propagated through the whole operator, generating
+ // uniform and structured logs.
+
+ // The constants specified here are zap specific
+ var logLevel zapcore.Level
+ logLevelVal, ok := os.LookupEnv("LOG_LEVEL")
+ if ok {
+ switch strings.ToLower(logLevelVal) {
+ case "error":
+ logLevel = zapcore.ErrorLevel
+ case "info":
+ logLevel = zapcore.InfoLevel
+ case "debug":
+ logLevel = zapcore.DebugLevel
+ default:
+ customLevel, err := strconv.Atoi(strings.ToLower(logLevelVal))
+ if err != nil {
+ return bm.Log, err, "Invalid log-level"
+ }
+ // Need to multiply by -1 to turn logr expected level into zap level
+ logLevel = zapcore.Level(int8(customLevel) * -1)
+ }
+ } else {
+ logLevel = zapcore.InfoLevel
+ }
+
+ // Use and set atomic level that all following log events are compared with
+ // in order to evaluate if a given log level on the event is enabled.
+ logf.SetLogger(zapctrl.New(func(o *zapctrl.Options) {
+ o.Development = false
+ o.Level = zap.NewAtomicLevelAt(logLevel)
+ }))
+
+ klog.SetLogger(bm.Log.AsLogger())
+
+ _, err := maxprocs.Set(maxprocs.Logger(func(f string, a ...interface{}) { bm.Log.Info(fmt.Sprintf(f, a)) }))
+
+ return bm.Log, err, "failed to set GOMAXPROCS from cgroups"
+}
+
+// getWatchNamespace returns the Namespace the operator should be watching for changes.
+func (bm BaseManager) GetWatchNamespace() (string, error) {
+ ns, found := os.LookupEnv(bm.WatchNamespaceEnvVar)
+ if !found {
+ return "", fmt.Errorf("%s must be set", bm.WatchNamespaceEnvVar)
+ }
+ return ns, nil
+}
+
+func (bm BaseManager) CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string) {
+ // Increase maximum burst that is used by client-side throttling,
+ // to prevent the requests made to apply the bundled Kamelets
+ // from being throttled.
+ cfg.QPS = 20
+ cfg.Burst = 200
+ bootstrapClient, err := client.NewClientWithConfig(false, cfg)
+ if err != nil {
+ return nil, err, "cannot initialize client"
+ }
+
+ return bootstrapClient, nil, ""
+}
+
+func (bm BaseManager) CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string) {
+ // We do not rely on the event broadcaster managed by controller runtime,
+ // so that we can check the operator has been granted permission to create
+ // Events. This is required for the operator to be installable by standard
+ // admin users, that are not granted create permission on Events by default.
+ broadcaster := record.NewBroadcaster()
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, corev1.GroupName, "events", watchNamespace, "", "create"); err != nil || !ok {
+ // Do not sink Events to the server as they'll be rejected
+ broadcaster = event.NewSinkLessBroadcaster(broadcaster)
+ return broadcaster, err, "cannot check permissions for creating Events"
+ bm.Log.Info("Event broadcasting is disabled because of missing permissions to create Events")
+ }
+
+ return broadcaster, nil, ""
+}
+
+func (bm BaseManager) GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string) {
+ operatorNamespace := platform.GetOperatorNamespace()
+ if operatorNamespace == "" {
+ // Fallback to using the watch namespace when the operator is not in-cluster.
+ // It does not support local (off-cluster) operator watching resources globally,
+ // in which case it's not possible to determine a namespace.
+ operatorNamespace = watchNamespace
+ if operatorNamespace == "" {
+ leaderElection = false
+ bm.Log.Info("unable to determine namespace for leader election")
+ }
+ }
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, coordination.GroupName, "leases", operatorNamespace, "", "create"); err != nil || !ok {
+ leaderElection = false
+ if err != nil {
+ return operatorNamespace, leaderElection, err, "cannot check permissions for creating Leases"
+ }
+ bm.Log.Info("The operator is not granted permissions to create Leases")
+ }
+
+ return operatorNamespace, leaderElection, nil, ""
+}
+
+// SetOperatorImage set the operator container image if it runs in-container
+func (bm BaseManager) SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string) {
+ var err error
+ platform.OperatorImage, err = getOperatorImage(bm.ControllerNamespace, bm.ControllerPodName, ctx, bootstrapClient)
+ return err, "cannot get operator container image"
+}
+
+// getOperatorImage returns the image currently used by the running operator if present (when running out of cluster, it may be absent).
+func getOperatorImage(namespace string, podName string, ctx context.Context, c ctrl.Reader) (string, error) {
Review Comment:
`context.Context` should be the first argument
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2049885062
:warning: Unit test coverage report - coverage decreased from 37.8% to 37.7% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2045101332
:warning: Unit test coverage report - coverage decreased from 37.8% to 37.7% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2059440873
:warning: Unit test coverage report - coverage decreased from 38% to 37.9% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505496052
##########
pkg/cmd/operator/operator.go:
##########
@@ -70,122 +51,48 @@ import (
var log = logutil.Log.WithName("cmd")
Review Comment:
may be better to have a better log name
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2007343706
> > I agree that we need some docs but since this PR is part of a larger issue and the rationale was briefly explained in [#3397 (comment)](https://github.com/apache/camel-k/issues/3397#issuecomment-1735525398), then I wonder if the doc could be something we do as a separate PR.
>
> In theory we want to release 2.3 by the end of the month. If we ends up merging this we'd include in 2.3 which, IIUC would not be either complete as still missing pieces (beside documentation). My suggestion would be to wait the release of 2.3 then, and to merge right after so you can focus on the rest of development to be ready for 2.4. Or, if you want to push this in 2.3 complete with the required documentation.
I think this is more for 3.x than 2.x, wonder if we should branch 2.x now and move main toward 3.x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2007314372
> I'm not complaining about having to run them locally, I'm just noticing the OSX build is almost always failing so I wonder what is the value of such test. Now I can run the test locally on my linux box and reporting them as ok but, what I've testes is not what has failed so people may spend time waiting and trying to find out why things are not working on OSX whereas it is an environment issue.
I've commented this aspect already some time ago in the comment: https://github.com/apache/camel-k/pull/5119#issuecomment-1983848486 apologies if it was not clear enough. I'm taking the task to notify this to any PR that may affect the Quarkus native checks. Unfortunately, while they are not fixed, the best way to handle this is to run them manually.
By the way, just to make this clear, I'm doing the same for my own PRs and I'll have to do this process at the time of cutting the release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "valdar (via GitHub)" <gi...@apache.org>.
valdar commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1976717697
> The main concern I have is about code duplication (maintainability) and performance loss. About the first, I think it would be a matter of abstracting the code from the operator and be able to call it from different places.
I have abstracted as much code as I could in the cmd/controller/manager.go interface and implementations. Hope it is a suitable solution.
> About performances I guess it's a bit more complicated. IIUC, so far we're having two separate Pods with 2 separate caches. I honestly don't think that the platform controller has to watch at any resource at all, but only owns the IntegrationPlatform resource. If it requires to watch all those resources for any reason, then, we should understand if we can share the cache somehow.
All not needed resources have been removed from watching in cache.
> Additionally, I wonder if instead of having a new deployment, we can simply have a sidecar container to take care of controlling this part.
@rinaldodev is already working on follow ups of this PR that requires the platform controller to be a separate operator, in fact the one that will bootstrap the camel k operators. So I think that is not worth to explore a sidecar option for the relative short time needed to complete that chunk of work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2061059222
:warning: Unit test coverage report - coverage decreased from 38% to 37.9% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2071772976
:warning: Unit test coverage report - coverage decreased from 38% to 37.9% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "valdar (via GitHub)" <gi...@apache.org>.
valdar commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1511236658
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,218 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "strconv"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ PrintVersion()
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error)
+ CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error)
+ GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error)
+ GetManagerOptions(bootstrapClient client.Client) (cache.Options, string, error)
+ CreateManager(ctx context.Context, healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options) (manager.Manager, client.Client, string, error)
+ ControllerPreStartResourcesInit(ctx context.Context, initCtx context.Context, bootstrapClient client.Client, controllerNamespace string, ctrlClient client.Client, mgr manager.Manager) (string, error)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+ Log logutil.Logger
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (string, error) {
+ errMessage, err := setMaxprocs(c.Log)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ c.Log.Debug("*** DEBUG level messages will be logged ***")
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return "cannot get client config", err
+ }
+ bootstrapClient, errMessage, err := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return errMessage, err
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, errMessage, err := c.ControllerManager.CreateEventBroadcaster(ctx, bootstrapClient)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return errMessage, err
+ }
+
+ controllerNamespace, leaderElection, errMessage, err := c.ControllerManager.GetControllerNamespaceAndLeaderElection(ctx, bootstrapClient, leaderElection)
+ if err != nil {
+ return errMessage, err
+ }
+ if !leaderElection {
+ c.Log.Info("Leader election is disabled!")
+ }
+
+ errMessage, err = setOperatorImage(ctx, bootstrapClient, controllerNamespace)
+ if err != nil {
+ return errMessage, err
+ }
+
+ options, errMessage, err := c.ControllerManager.GetManagerOptions(bootstrapClient)
+ if err != nil {
+ return errMessage, err
+ }
+
+ mgr, ctrlClient, errMessage, err := c.ControllerManager.CreateManager(ctx, healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options)
+ if err != nil {
+ return errMessage, err
+ }
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ errMessage, err = c.ControllerManager.ControllerPreStartResourcesInit(ctx, initCtx, bootstrapClient, controllerNamespace, ctrlClient, mgr)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.Log.Info("Starting the manager")
+ return "manager exited non-zero", mgr.Start(ctx)
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespace string
+ ControllerNamespace string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error) {
+ // Increase maximum burst that is used by client-side throttling,
+ // to prevent the requests made to apply the bundled Kamelets
+ // from being throttled.
+ cfg.QPS = 20
+ cfg.Burst = 200
+ bootstrapClient, err := client.NewClientWithConfig(false, cfg)
+ if err != nil {
+ return nil, "cannot initialize client", err
+ }
+
+ return bootstrapClient, "", nil
+}
+
+func (bm BaseManager) CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error) {
+ // We do not rely on the event broadcaster managed by controller runtime,
+ // so that we can check the operator has been granted permission to create
+ // Events. This is required for the operator to be installable by standard
+ // admin users, that are not granted create permission on Events by default.
+ broadcaster := record.NewBroadcaster()
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, corev1.GroupName, "events", bm.WatchNamespace, "", "create"); err != nil || !ok {
+ // Do not sink Events to the server as they'll be rejected
+ broadcaster = event.NewSinkLessBroadcaster(broadcaster)
+ if err != nil {
+ bm.Log.Info("Event broadcasting is disabled because of missing permissions to create Events")
+ }
+ return broadcaster, "cannot check permissions for creating Events", err
+ }
+
+ return broadcaster, "", nil
+}
+
+func (bm BaseManager) GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error) {
+ controllerNamespace := bm.ControllerNamespace
+ if controllerNamespace == "" {
+ // Fallback to using the watch namespace when the operator is not in-cluster.
+ // It does not support local (off-cluster) operator watching resources globally,
+ // in which case it's not possible to determine a namespace.
+ controllerNamespace = bm.WatchNamespace
+ if controllerNamespace == "" {
+ leaderElection = false
+ bm.Log.Info("unable to determine namespace for leader election")
+ }
+ }
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, coordination.GroupName, "leases", controllerNamespace, "", "create"); err != nil || !ok {
Review Comment:
I think you addressed that opening this issue https://github.com/apache/camel-k/issues/5210
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1511126823
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,218 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "strconv"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ PrintVersion()
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error)
+ CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error)
+ GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error)
+ GetManagerOptions(bootstrapClient client.Client) (cache.Options, string, error)
+ CreateManager(ctx context.Context, healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options) (manager.Manager, client.Client, string, error)
+ ControllerPreStartResourcesInit(ctx context.Context, initCtx context.Context, bootstrapClient client.Client, controllerNamespace string, ctrlClient client.Client, mgr manager.Manager) (string, error)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+ Log logutil.Logger
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (string, error) {
+ errMessage, err := setMaxprocs(c.Log)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ c.Log.Debug("*** DEBUG level messages will be logged ***")
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return "cannot get client config", err
+ }
+ bootstrapClient, errMessage, err := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return errMessage, err
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, errMessage, err := c.ControllerManager.CreateEventBroadcaster(ctx, bootstrapClient)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return errMessage, err
+ }
+
+ controllerNamespace, leaderElection, errMessage, err := c.ControllerManager.GetControllerNamespaceAndLeaderElection(ctx, bootstrapClient, leaderElection)
+ if err != nil {
+ return errMessage, err
+ }
+ if !leaderElection {
+ c.Log.Info("Leader election is disabled!")
+ }
+
+ errMessage, err = setOperatorImage(ctx, bootstrapClient, controllerNamespace)
+ if err != nil {
+ return errMessage, err
+ }
+
+ options, errMessage, err := c.ControllerManager.GetManagerOptions(bootstrapClient)
+ if err != nil {
+ return errMessage, err
+ }
+
+ mgr, ctrlClient, errMessage, err := c.ControllerManager.CreateManager(ctx, healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options)
+ if err != nil {
+ return errMessage, err
+ }
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ errMessage, err = c.ControllerManager.ControllerPreStartResourcesInit(ctx, initCtx, bootstrapClient, controllerNamespace, ctrlClient, mgr)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.Log.Info("Starting the manager")
+ return "manager exited non-zero", mgr.Start(ctx)
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespace string
+ ControllerNamespace string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error) {
+ // Increase maximum burst that is used by client-side throttling,
+ // to prevent the requests made to apply the bundled Kamelets
+ // from being throttled.
+ cfg.QPS = 20
+ cfg.Burst = 200
+ bootstrapClient, err := client.NewClientWithConfig(false, cfg)
+ if err != nil {
+ return nil, "cannot initialize client", err
+ }
+
+ return bootstrapClient, "", nil
+}
+
+func (bm BaseManager) CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error) {
+ // We do not rely on the event broadcaster managed by controller runtime,
+ // so that we can check the operator has been granted permission to create
+ // Events. This is required for the operator to be installable by standard
+ // admin users, that are not granted create permission on Events by default.
+ broadcaster := record.NewBroadcaster()
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, corev1.GroupName, "events", bm.WatchNamespace, "", "create"); err != nil || !ok {
Review Comment:
This is not something that is part of the PR but it looks like that we have some roles & binding for grating the operator access to `events` so wonder if we really need to have our own broadcaster instead of relying on the default one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "valdar (via GitHub)" <gi...@apache.org>.
valdar commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1511438324
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,218 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "strconv"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ PrintVersion()
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error)
+ CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error)
+ GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error)
+ GetManagerOptions(bootstrapClient client.Client) (cache.Options, string, error)
+ CreateManager(ctx context.Context, healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options) (manager.Manager, client.Client, string, error)
+ ControllerPreStartResourcesInit(ctx context.Context, initCtx context.Context, bootstrapClient client.Client, controllerNamespace string, ctrlClient client.Client, mgr manager.Manager) (string, error)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+ Log logutil.Logger
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505456692
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ LoggerSetup() (logutil.Logger, error, string)
+ PrintVersion()
+ GetWatchNamespace() (string, error)
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string)
+ CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string)
+ GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string)
+ SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string)
+ GetManagerOptions(bootstrapClient client.Client, watchNamespace string) (cache.Options, error, string)
+ CreateManager(healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options, ctx context.Context) (manager.Manager, client.Client, error, string)
+ ControllerPreStartResourcesInit(initCtx context.Context, bootstrapClient client.Client, watchNamespace string, controllerNamespace string, ctx context.Context, ctrlClient client.Client, mgr manager.Manager) (error, string)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (error, string) {
+ log, err, errMessage := c.ControllerManager.LoggerSetup()
+ if err != nil {
+ log.Error(err, errMessage)
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ log.Debug("*** DEBUG level messages will be logged ***")
+
+ watchNamespace, err := c.ControllerManager.GetWatchNamespace()
+ if err != nil {
+ return err, "failed to get watch namespace"
+ }
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return err, "cannot get client config"
+ }
+
+ bootstrapClient, err, errMessage := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return err, errMessage
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, err, errMessage := c.ControllerManager.CreateEventBroadcaster(bootstrapClient, watchNamespace, ctx)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return err, errMessage
+ }
+
+ controllerNamespace, leaderElection, err, errMessage := c.ControllerManager.GetControllerNamespaceAndLeaderElection(watchNamespace, bootstrapClient, leaderElection, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+ if !leaderElection {
+ log.Info("Leader election is disabled!")
+ }
+
+ err, errMessage = c.ControllerManager.SetOperatorImage(bootstrapClient, ctx)
+ if err != nil {
+ return err, errMessage
+ }
+
+ options, err, errMessage := c.ControllerManager.GetManagerOptions(bootstrapClient, watchNamespace)
+ if err != nil {
+ return err, errMessage
+ }
+
+ mgr, ctrlClient, err, errMessage := c.ControllerManager.CreateManager(healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options, ctx)
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ err, errMessage = c.ControllerManager.ControllerPreStartResourcesInit(initCtx, bootstrapClient, watchNamespace, controllerNamespace, ctx, ctrlClient, mgr)
+ if err != nil {
+ return err, errMessage
+ }
+
+ log.Info("Starting the manager")
+ return mgr.Start(ctx), "manager exited non-zero"
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespaceEnvVar string
+ ControllerNamespace string
+ ControllerPodName string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) LoggerSetup() (logutil.Logger, error, string) {
+ // The logger instantiated here can be changed to any logger
+ // implementing the logr.Logger interface. This logger will
+ // be propagated through the whole operator, generating
+ // uniform and structured logs.
+
+ // The constants specified here are zap specific
+ var logLevel zapcore.Level
+ logLevelVal, ok := os.LookupEnv("LOG_LEVEL")
+ if ok {
+ switch strings.ToLower(logLevelVal) {
+ case "error":
+ logLevel = zapcore.ErrorLevel
+ case "info":
+ logLevel = zapcore.InfoLevel
+ case "debug":
+ logLevel = zapcore.DebugLevel
+ default:
+ customLevel, err := strconv.Atoi(strings.ToLower(logLevelVal))
+ if err != nil {
+ return bm.Log, err, "Invalid log-level"
+ }
+ // Need to multiply by -1 to turn logr expected level into zap level
+ logLevel = zapcore.Level(int8(customLevel) * -1)
+ }
+ } else {
+ logLevel = zapcore.InfoLevel
+ }
+
+ // Use and set atomic level that all following log events are compared with
+ // in order to evaluate if a given log level on the event is enabled.
+ logf.SetLogger(zapctrl.New(func(o *zapctrl.Options) {
+ o.Development = false
+ o.Level = zap.NewAtomicLevelAt(logLevel)
+ }))
+
+ klog.SetLogger(bm.Log.AsLogger())
+
+ _, err := maxprocs.Set(maxprocs.Logger(func(f string, a ...interface{}) { bm.Log.Info(fmt.Sprintf(f, a)) }))
+
+ return bm.Log, err, "failed to set GOMAXPROCS from cgroups"
+}
+
+// getWatchNamespace returns the Namespace the operator should be watching for changes.
+func (bm BaseManager) GetWatchNamespace() (string, error) {
+ ns, found := os.LookupEnv(bm.WatchNamespaceEnvVar)
+ if !found {
+ return "", fmt.Errorf("%s must be set", bm.WatchNamespaceEnvVar)
+ }
+ return ns, nil
+}
+
+func (bm BaseManager) CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string) {
+ // Increase maximum burst that is used by client-side throttling,
+ // to prevent the requests made to apply the bundled Kamelets
+ // from being throttled.
+ cfg.QPS = 20
+ cfg.Burst = 200
+ bootstrapClient, err := client.NewClientWithConfig(false, cfg)
+ if err != nil {
+ return nil, err, "cannot initialize client"
+ }
+
+ return bootstrapClient, nil, ""
+}
+
+func (bm BaseManager) CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string) {
+ // We do not rely on the event broadcaster managed by controller runtime,
+ // so that we can check the operator has been granted permission to create
+ // Events. This is required for the operator to be installable by standard
+ // admin users, that are not granted create permission on Events by default.
+ broadcaster := record.NewBroadcaster()
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, corev1.GroupName, "events", watchNamespace, "", "create"); err != nil || !ok {
+ // Do not sink Events to the server as they'll be rejected
+ broadcaster = event.NewSinkLessBroadcaster(broadcaster)
+ return broadcaster, err, "cannot check permissions for creating Events"
+ bm.Log.Info("Event broadcasting is disabled because of missing permissions to create Events")
+ }
+
+ return broadcaster, nil, ""
+}
+
+func (bm BaseManager) GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string) {
+ operatorNamespace := platform.GetOperatorNamespace()
+ if operatorNamespace == "" {
+ // Fallback to using the watch namespace when the operator is not in-cluster.
+ // It does not support local (off-cluster) operator watching resources globally,
+ // in which case it's not possible to determine a namespace.
+ operatorNamespace = watchNamespace
+ if operatorNamespace == "" {
+ leaderElection = false
+ bm.Log.Info("unable to determine namespace for leader election")
+ }
+ }
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, coordination.GroupName, "leases", operatorNamespace, "", "create"); err != nil || !ok {
+ leaderElection = false
+ if err != nil {
+ return operatorNamespace, leaderElection, err, "cannot check permissions for creating Leases"
+ }
+ bm.Log.Info("The operator is not granted permissions to create Leases")
+ }
+
+ return operatorNamespace, leaderElection, nil, ""
+}
+
+// SetOperatorImage set the operator container image if it runs in-container
+func (bm BaseManager) SetOperatorImage(bootstrapClient client.Client, ctx context.Context) (error, string) {
Review Comment:
it seems this function does not really belong to the `Manager` interface as in fact it sets the image elsewhere. It may be moved to a simple function so it may simplify the `Manager` interface.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1473236743
##########
pkg/cmd/platformcontroller/platformcontroller.go:
##########
@@ -0,0 +1,272 @@
+/*
Review Comment:
I see a lot of duplicated code. I think it would be wiser to abstract the code in common with the operator executable and minimize the code to maintain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2006770214
> Also, several tests have failed and they don't seem to be the usual flaky ones. I've restarted them.
The native tests seems to be failing constantly on OSX, can't we skip testing native images on OSX ? given not everyone has an OXS I wonder how actually one could run them locally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2006739926
> @christophd @squakez I guess this is now in a good shape to be merged
I haven't received any feedback about my last 2 comments. We need to run manually the native checks and it would be nice some documentation or explanation that the final user will require for sure when moving to a new deployment model. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1504936589
##########
build/test.log:
##########
Review Comment:
this is likely a file we don't want to include
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505494747
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
Review Comment:
don't mind, was too quick
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1505497518
##########
pkg/cmd/platformcontroller/platformcontroller.go:
##########
@@ -0,0 +1,89 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package platformcontroller
+
+import (
+ "context"
+ "flag"
+ "fmt"
+ "os"
+ "runtime"
+
+ "github.com/apache/camel-k/v2/pkg/client"
+ managerCmd "github.com/apache/camel-k/v2/pkg/cmd/manager"
+ "github.com/apache/camel-k/v2/pkg/controller"
+ "github.com/apache/camel-k/v2/pkg/install"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/defaults"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+)
+
+var log = logutil.Log.WithName("cmd")
Review Comment:
may be better to use a better log name
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1947492219
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.7% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1926467434
> What about these tests:
>
> ❌ TestMetrics (3m0.27s) ❌ TestMetrics/reconciliation_duration_metric (10ms)
>
> they seems flaky to me, first run they were passing, they are passing locally, but now failing in CI :/
>
> They are also marked as "problematic" but CI runs without the skip problematic config? :/
No, I don't think they are flaky. It fails at line:
```
Expect(platformReconciled).NotTo(BeNil())
```
I suspect that the operator perform and provide metrics which we may have lost if running the reconciliation loop outside the main process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2004577345
:warning: Unit test coverage report - coverage decreased from 37.2% to 37.1% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2006879615
@lburgazzoli the problem is not the failure of the checks per se. As we've done in other PR, everything that may affect the Quarkus native has to be validated manually in order to make sure we don't introduce any regression. Please, have a look at this PR for instance: https://github.com/apache/camel-k/pull/5234#issuecomment-1985493127
If the folks have any problem running the native tests, you or I can run them and publish here the results, although, as suggested to some other contributor, it would be good to be able to do it on their own as it's should be a normal practice to do when doing some development. Running locally it is as easy as running any other integration test [1], it is only taking a bit of time more.
Beside, that normal test have failed and I've restarted, so, we need `common` to finish and be green.
Beside, as mentioned in [2] my personal feeling is that these kind of changes would require some documentation and some rationale explanation as the users that will move from 2.2 to 2.3 will have the surprise of a multi deployment that may require them to adjust monitoring and other operational aspects. To me this is a breaking change, but if you want to proceed, at least it has to be clearly explained to the final users.
[1] https://camel.apache.org/camel-k/2.2.x/contributing/e2e.html#testing-e2e-structure
[2] https://github.com/apache/camel-k/pull/5119#issuecomment-1976919321
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2006809347
> > > Also, several tests have failed and they don't seem to be the usual flaky ones. I've restarted them.
> >
> >
> > The native tests seems to be failing constantly on OSX, can't we skip testing native images on OSX ? given not everyone has an OXS I wonder how actually one could run them locally
>
> They can run on any OS. The Macos is only on github actions. We are executing in our local machines while we don't have a fix on the GH infra.
Can we maybe disable native OSX tests on CI till we have a fix ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2006783913
> > Also, several tests have failed and they don't seem to be the usual flaky ones. I've restarted them.
>
> The native tests seems to be failing constantly on OSX, can't we skip testing native images on OSX ? given not everyone has an OXS I wonder how actually one could run them locally
They can run on any OS. The Macos is only on github actions. We are executing in our local machines while we don't have a fix on the GH infra.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2006711589
@christophd @squakez I guess this is now in a good shape to be merged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2009111979
@valdar please rebase against `main` again. The Quarkus native checks have been fixed, so you won't need to run them manually any longer. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1510737986
##########
e2e/support/test_support.go:
##########
@@ -2245,7 +2253,7 @@ func OperatorPod(ns string) func() *corev1.Pod {
if err := TestClient().List(TestContext, &lst,
ctrl.InNamespace(ns),
ctrl.MatchingLabels{
- "camel.apache.org/component": "operator",
+ "camel.apache.org/component": componentLabelValue,
Review Comment:
nitpick: as a further cleanup, we should probably deprecate some of the `camel.apache.org` annotation in favor of k8s's [recommended labels ](https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1510901240
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,218 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "strconv"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ PrintVersion()
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error)
+ CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error)
+ GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error)
+ GetManagerOptions(bootstrapClient client.Client) (cache.Options, string, error)
+ CreateManager(ctx context.Context, healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options) (manager.Manager, client.Client, string, error)
+ ControllerPreStartResourcesInit(ctx context.Context, initCtx context.Context, bootstrapClient client.Client, controllerNamespace string, ctrlClient client.Client, mgr manager.Manager) (string, error)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+ Log logutil.Logger
+}
+
+func (c ControllerCmd) Run(healthPort, monitoringPort int32, leaderElection bool, leaderElectionID string) (string, error) {
+ errMessage, err := setMaxprocs(c.Log)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.ControllerManager.PrintVersion()
+ // Will only appear if DEBUG level has been enabled using the env var LOG_LEVEL
+ c.Log.Debug("*** DEBUG level messages will be logged ***")
+
+ cfg, err := config.GetConfig()
+ if err != nil {
+ return "cannot get client config", err
+ }
+ bootstrapClient, errMessage, err := c.ControllerManager.CreateBootstrapClient(cfg)
+ if err != nil {
+ return errMessage, err
+ }
+
+ ctx := signals.SetupSignalHandler()
+
+ eventBroadcaster, errMessage, err := c.ControllerManager.CreateEventBroadcaster(ctx, bootstrapClient)
+ if eventBroadcaster != nil {
+ defer eventBroadcaster.Shutdown()
+ }
+ if err != nil {
+ return errMessage, err
+ }
+
+ controllerNamespace, leaderElection, errMessage, err := c.ControllerManager.GetControllerNamespaceAndLeaderElection(ctx, bootstrapClient, leaderElection)
+ if err != nil {
+ return errMessage, err
+ }
+ if !leaderElection {
+ c.Log.Info("Leader election is disabled!")
+ }
+
+ errMessage, err = setOperatorImage(ctx, bootstrapClient, controllerNamespace)
+ if err != nil {
+ return errMessage, err
+ }
+
+ options, errMessage, err := c.ControllerManager.GetManagerOptions(bootstrapClient)
+ if err != nil {
+ return errMessage, err
+ }
+
+ mgr, ctrlClient, errMessage, err := c.ControllerManager.CreateManager(ctx, healthPort, monitoringPort, leaderElection, leaderElectionID, cfg, eventBroadcaster, controllerNamespace, options)
+ if err != nil {
+ return errMessage, err
+ }
+
+ initCtx, initCancel := context.WithTimeout(ctx, 1*time.Minute)
+ defer initCancel()
+
+ errMessage, err = c.ControllerManager.ControllerPreStartResourcesInit(ctx, initCtx, bootstrapClient, controllerNamespace, ctrlClient, mgr)
+ if err != nil {
+ return errMessage, err
+ }
+
+ c.Log.Info("Starting the manager")
+ return "manager exited non-zero", mgr.Start(ctx)
+}
+
+type BaseManager struct {
+ Log logutil.Logger
+ WatchNamespace string
+ ControllerNamespace string
+ AddToManager func(ctx context.Context, manager manager.Manager, client client.Client) error
+}
+
+func (bm BaseManager) CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error) {
+ // Increase maximum burst that is used by client-side throttling,
+ // to prevent the requests made to apply the bundled Kamelets
+ // from being throttled.
+ cfg.QPS = 20
+ cfg.Burst = 200
+ bootstrapClient, err := client.NewClientWithConfig(false, cfg)
+ if err != nil {
+ return nil, "cannot initialize client", err
+ }
+
+ return bootstrapClient, "", nil
+}
+
+func (bm BaseManager) CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error) {
+ // We do not rely on the event broadcaster managed by controller runtime,
+ // so that we can check the operator has been granted permission to create
+ // Events. This is required for the operator to be installable by standard
+ // admin users, that are not granted create permission on Events by default.
+ broadcaster := record.NewBroadcaster()
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, corev1.GroupName, "events", bm.WatchNamespace, "", "create"); err != nil || !ok {
+ // Do not sink Events to the server as they'll be rejected
+ broadcaster = event.NewSinkLessBroadcaster(broadcaster)
+ if err != nil {
+ bm.Log.Info("Event broadcasting is disabled because of missing permissions to create Events")
+ }
+ return broadcaster, "cannot check permissions for creating Events", err
+ }
+
+ return broadcaster, "", nil
+}
+
+func (bm BaseManager) GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error) {
+ controllerNamespace := bm.ControllerNamespace
+ if controllerNamespace == "" {
+ // Fallback to using the watch namespace when the operator is not in-cluster.
+ // It does not support local (off-cluster) operator watching resources globally,
+ // in which case it's not possible to determine a namespace.
+ controllerNamespace = bm.WatchNamespace
+ if controllerNamespace == "" {
+ leaderElection = false
+ bm.Log.Info("unable to determine namespace for leader election")
+ }
+ }
+
+ if ok, err := kubernetes.CheckPermission(ctx, bootstrapClient, coordination.GroupName, "leases", controllerNamespace, "", "create"); err != nil || !ok {
Review Comment:
This is not something that is part of the PR but it dates back to 2021 and was probably done because support for leases was not yet common. I think that given that we are explicitly bind the permissions to the operator, we could probably remove it in a future iteration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1510746942
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,218 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "strconv"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ PrintVersion()
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, string, error)
+ CreateEventBroadcaster(ctx context.Context, bootstrapClient client.Client) (record.EventBroadcaster, string, error)
+ GetControllerNamespaceAndLeaderElection(ctx context.Context, bootstrapClient client.Client, leaderElection bool) (string, bool, string, error)
+ GetManagerOptions(bootstrapClient client.Client) (cache.Options, string, error)
+ CreateManager(ctx context.Context, healthPort int32, monitoringPort int32, leaderElection bool, leaderElectionID string, cfg *rest.Config, eventBroadcaster record.EventBroadcaster, controllerNamespace string, options cache.Options) (manager.Manager, client.Client, string, error)
+ ControllerPreStartResourcesInit(ctx context.Context, initCtx context.Context, bootstrapClient client.Client, controllerNamespace string, ctrlClient client.Client, mgr manager.Manager) (string, error)
+}
+
+type ControllerCmd struct {
+ ControllerManager Manager
+ Log logutil.Logger
Review Comment:
nitpick: those variable can probably be made private
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2007299761
> I agree that we need some docs but since this PR is part of a larger issue and the rationale was briefly explained in [#3397 (comment)](https://github.com/apache/camel-k/issues/3397#issuecomment-1735525398), then I wonder if the doc could be something we do as a separate PR.
In theory we want to release 2.3 by the end of the month. If we ends up merging this we'd include in 2.3 which, IIUC would not be either complete as still missing pieces (beside documentation). My suggestion would be to wait the release of 2.3 then, and to merge right after so you can focus on the rest of development to be ready for 2.4. Or, if you want to push this in 2.3 complete with the required documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2007558011
> > > I agree that we need some docs but since this PR is part of a larger issue and the rationale was briefly explained in [#3397 (comment)](https://github.com/apache/camel-k/issues/3397#issuecomment-1735525398), then I wonder if the doc could be something we do as a separate PR.
> >
> >
> > In theory we want to release 2.3 by the end of the month. If we ends up merging this we'd include in 2.3 which, IIUC would not be either complete as still missing pieces (beside documentation). My suggestion would be to wait the release of 2.3 then, and to merge right after so you can focus on the rest of development to be ready for 2.4. Or, if you want to push this in 2.3 complete with the required documentation.
>
> I think this is more for 3.x than 2.x, wonder if we should branch 2.x now and move main toward 3.x
No problem for me. Mind that there may be some work to perform on the actions as well (nightly upgrades, releases, etc...) and likely this has to be communicated and accepted by the community before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1470872609
##########
config/manager/platformcontroller-deployment.yaml:
##########
@@ -0,0 +1,89 @@
+# ---------------------------------------------------------------------------
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+# ---------------------------------------------------------------------------
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: camel-k-platformcontroller
+ labels:
+ app: "camel-k"
+ camel.apache.org/component: operator
+ name: camel-k-platformcontroller
+ app.kubernetes.io/component: operator
+ app.kubernetes.io/name: camel-k
+ app.kubernetes.io/version: "2.2.0-SNAPSHOT"
+spec:
+ replicas: 1
+ strategy:
+ type: Recreate
+ selector:
+ matchLabels:
+ name: camel-k-platformcontroller
+ template:
+ metadata:
+ labels:
+ name: camel-k-platformcontroller
+ camel.apache.org/component: operator
+ app: "camel-k"
+ app.kubernetes.io/component: operator
+ app.kubernetes.io/name: camel-k
+ app.kubernetes.io/version: "2.2.0-SNAPSHOT"
+ spec:
+ serviceAccountName: camel-k-operator
+ containers:
+ - name: camel-k-platformcontroller
+ image: docker.io/apache/camel-k:2.2.0-SNAPSHOT
Review Comment:
```suggestion
image: docker.io/apache/camel-k:2.3.0-SNAPSHOT
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1966096570
@valdar @rinaldodev mind rebasing ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1976919321
@valdar I don't have enough context to review this one, sorry. The immediate impression I have is that from community perspective it would be good to have some design diagram and some user documentation to illustrate how this new model works, above all what it implies in terms of resources, monitoring and in general any operational aspect that it would be required as we move to a new approach with multiple Deployments. Also some explanation on the "why" this work is beneficial may be helpful to be eventually welcomed by any user that may not agree with this new approach and such a change that is going to happen on a minor release version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#discussion_r1504941598
##########
pkg/cmd/manager/controller.go:
##########
@@ -0,0 +1,310 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manager
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strconv"
+ "strings"
+ "time"
+
+ "github.com/apache/camel-k/v2/pkg/apis"
+ "github.com/apache/camel-k/v2/pkg/client"
+ "github.com/apache/camel-k/v2/pkg/event"
+ "github.com/apache/camel-k/v2/pkg/platform"
+ "github.com/apache/camel-k/v2/pkg/util/kubernetes"
+ logutil "github.com/apache/camel-k/v2/pkg/util/log"
+ "go.uber.org/automaxprocs/maxprocs"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+ coordination "k8s.io/api/coordination/v1"
+ corev1 "k8s.io/api/core/v1"
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/klog/v2"
+ "sigs.k8s.io/controller-runtime/pkg/cache"
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/config"
+ "sigs.k8s.io/controller-runtime/pkg/healthz"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ zapctrl "sigs.k8s.io/controller-runtime/pkg/log/zap"
+ "sigs.k8s.io/controller-runtime/pkg/manager"
+ "sigs.k8s.io/controller-runtime/pkg/manager/signals"
+)
+
+type Manager interface {
+ LoggerSetup() (logutil.Logger, error, string)
+ PrintVersion()
+ GetWatchNamespace() (string, error)
+ CreateBootstrapClient(cfg *rest.Config) (client.Client, error, string)
+ CreateEventBroadcaster(bootstrapClient client.Client, watchNamespace string, ctx context.Context) (record.EventBroadcaster, error, string)
+ GetControllerNamespaceAndLeaderElection(watchNamespace string, bootstrapClient client.Client, leaderElection bool, ctx context.Context) (string, bool, error, string)
Review Comment:
by convention, `context.Context` is usually the first argument
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-1983848486
I've enabled Quarkus Native tests. However it's very likely they fail due to some infrastructure problem that we have not yet tackled. In such circumstances, please, run the Quarkus Native test manually in your local environment and report result here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix #4948 part of #3397 [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5119:
URL: https://github.com/apache/camel-k/pull/5119#issuecomment-2064617439
:warning: Unit test coverage report - coverage decreased from 38% to 37.9% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org