From f3ecfbbdb58147e9823b0310df66b01d545650a7 Mon Sep 17 00:00:00 2001 From: Chris Doherty Date: Sat, 25 Mar 2023 21:00:46 -0500 Subject: [PATCH 1/2] Replace packethost/pkg/logger with logr Signed-off-by: Chris Doherty --- cmd/tink-controller/main.go | 20 ++---- cmd/tink-server/main.go | 62 ++++++++--------- cmd/tink-worker/cmd/root.go | 22 ++++-- cmd/tink-worker/main.go | 19 +----- cmd/tink-worker/worker/container_manager.go | 23 +++---- .../worker/container_manager_test.go | 13 ++-- cmd/tink-worker/worker/log_capturer.go | 15 ++--- cmd/tink-worker/worker/log_capturer_test.go | 15 +++-- cmd/tink-worker/worker/registry.go | 2 +- cmd/tink-worker/worker/registry_test.go | 5 +- cmd/tink-worker/worker/worker.go | 67 +++++++++---------- cmd/virtual-worker/cmd/root.go | 22 ++++-- cmd/virtual-worker/main.go | 12 +--- .../worker/container_manager.go | 18 ++--- go.mod | 20 +++--- go.sum | 1 - internal/e2e/e2e_test.go | 2 +- internal/e2e/tink_suite_test.go | 16 ++--- internal/httpserver/http_server.go | 10 +-- internal/server/kubernetes_api.go | 8 +-- internal/server/kubernetes_api_test.go | 5 +- internal/server/kubernetes_api_workflow.go | 10 +-- 22 files changed, 180 insertions(+), 207 deletions(-) diff --git a/cmd/tink-controller/main.go b/cmd/tink-controller/main.go index e95eb5556..556bdaaf3 100644 --- a/cmd/tink-controller/main.go +++ b/cmd/tink-controller/main.go @@ -1,14 +1,12 @@ package main import ( - "context" "fmt" "os" "strings" "github.com/go-logr/logr" "github.com/go-logr/zapr" - "github.com/packethost/pkg/log" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -44,28 +42,20 @@ func (c *Config) AddFlags(fs *pflag.FlagSet) { } func main() { - // Init the packet logger as its used throughout the codebase. - logger, err := log.Init("github.com/tinkerbell/tink") - if err != nil { - panic(err) - } - cmd := NewRootCommand() - if err := cmd.ExecuteContext(context.Background()); err != nil { - logger.Close() - fmt.Fprint(os.Stderr, err.Error()) + if err := cmd.Execute(); err != nil { os.Exit(1) } - logger.Close() } func NewRootCommand() *cobra.Command { - config := &Config{} - zapLogger, err := zap.NewProduction() + var config Config + + zlog, err := zap.NewProduction() if err != nil { panic(err) } - logger := zapr.NewLogger(zapLogger) + logger := zapr.NewLogger(zlog).WithName("github.com/tinkerbell/tink") cmd := &cobra.Command{ Use: "tink-controller", diff --git a/cmd/tink-server/main.go b/cmd/tink-server/main.go index 98aca3f3a..1331a4dfd 100644 --- a/cmd/tink-server/main.go +++ b/cmd/tink-server/main.go @@ -9,23 +9,24 @@ import ( "syscall" "github.com/equinix-labs/otel-init-go/otelinit" + "github.com/go-logr/logr" + "github.com/go-logr/zapr" "github.com/packethost/pkg/env" - "github.com/packethost/pkg/log" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/tinkerbell/tink/internal/grpcserver" "github.com/tinkerbell/tink/internal/httpserver" "github.com/tinkerbell/tink/internal/server" + "go.uber.org/zap" ) // version is set at build time. var version = "devel" -// DaemonConfig represents all the values you can configure as part of the tink-server. +// Config represents all the values you can configure as part of the tink-server. // You can change the configuration via environment variable, or file, or command flags. -type DaemonConfig struct { - Facility string +type Config struct { GRPCAuthority string HTTPAuthority string Backend string @@ -41,8 +42,7 @@ func backends() []string { return []string{backendKubernetes} } -func (c *DaemonConfig) AddFlags(fs *pflag.FlagSet) { - fs.StringVar(&c.Facility, "facility", "deprecated", "This is temporary. It will be removed") +func (c *Config) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&c.GRPCAuthority, "grpc-authority", ":42113", "The address used to expose the gRPC server") fs.StringVar(&c.HTTPAuthority, "http-authority", ":42114", "The address used to expose the HTTP server") fs.StringVar(&c.Backend, "backend", backendKubernetes, fmt.Sprintf("The backend datastore to use. Must be one of %s", strings.Join(backends(), ", "))) @@ -51,33 +51,27 @@ func (c *DaemonConfig) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&c.KubeNamespace, "kube-namespace", "", "The Kubernetes namespace to target") } -func (c *DaemonConfig) PopulateFromLegacyEnvVar() { - c.Facility = env.Get("FACILITY", c.Facility) +func (c *Config) PopulateFromLegacyEnvVar() { c.GRPCAuthority = env.Get("TINKERBELL_GRPC_AUTHORITY", c.GRPCAuthority) c.HTTPAuthority = env.Get("TINKERBELL_HTTP_AUTHORITY", c.HTTPAuthority) } func main() { - logger, err := log.Init("github.com/tinkerbell/tink") - if err != nil { - panic(err) + if err := NewRootCommand().Execute(); err != nil { + fmt.Fprint(os.Stderr, err.Error()) + os.Exit(1) } +} - ctx := context.Background() - ctx, otelShutdown := otelinit.InitOpenTelemetry(ctx, "github.com/tinkerbell/tink") - - config := &DaemonConfig{} +func NewRootCommand() *cobra.Command { + var config Config - cmd := NewRootCommand(config, logger) - if err := cmd.ExecuteContext(ctx); err != nil { - os.Exit(1) + zlog, err := zap.NewProduction() + if err != nil { + panic(err) } + logger := zapr.NewLogger(zlog).WithName("github.com/tinkerbell/tink") - logger.Close() - otelShutdown(ctx) -} - -func NewRootCommand(config *DaemonConfig, logger log.Logger) *cobra.Command { cmd := &cobra.Command{ Use: "tink-server", PreRunE: func(cmd *cobra.Command, args []string) error { @@ -95,11 +89,14 @@ func NewRootCommand(config *DaemonConfig, logger log.Logger) *cobra.Command { // the old way works as before. config.PopulateFromLegacyEnvVar() - logger.Info("starting version " + version) + logger.Info("Starting version " + version) + + ctx, oshutdown := otelinit.InitOpenTelemetry(cmd.Context(), "github.com/tinkerbell/tink") + defer oshutdown(context.Background()) sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGINT, syscall.SIGQUIT, syscall.SIGTERM) - ctx, closer := context.WithCancel(cmd.Context()) + ctx, closer := context.WithCancel(ctx) defer closer() // TODO(gianarb): I think we can do better in terms of // graceful shutdown and error management but I want to @@ -133,15 +130,15 @@ func NewRootCommand(config *DaemonConfig, logger log.Logger) *cobra.Command { if err != nil { return err } - logger.With("address", addr).Info("started listener") + logger.Info("started listener", "address", addr) httpserver.SetupHTTP(ctx, logger, config.HTTPAuthority, errCh) select { case err := <-errCh: - logger.Error(err) + logger.Error(err, "") case sig := <-sigs: - logger.With("signal", sig.String()).Info("signal received, stopping servers") + logger.Info("signal received, stopping servers", "signal", sig.String()) closer() } @@ -161,7 +158,7 @@ func NewRootCommand(config *DaemonConfig, logger log.Logger) *cobra.Command { return cmd } -func createViper(logger log.Logger) (*viper.Viper, error) { +func createViper(log logr.Logger) (*viper.Viper, error) { v := viper.New() v.AutomaticEnv() v.SetConfigName("tink-server") @@ -172,12 +169,11 @@ func createViper(logger log.Logger) (*viper.Viper, error) { // If a config file is found, read it in. if err := v.ReadInConfig(); err != nil { if _, ok := err.(viper.ConfigFileNotFoundError); !ok { - logger.With("configFile", v.ConfigFileUsed()).Error(err, "could not load config file") - return nil, err + return nil, fmt.Errorf("could not load config file: %w", err) } - logger.Info("no config file found") + log.Info("No config file found") } else { - logger.With("configFile", v.ConfigFileUsed()).Info("loaded config file") + log.Info("Loaded config file", "path", v.ConfigFileUsed()) } return v, nil diff --git a/cmd/tink-worker/cmd/root.go b/cmd/tink-worker/cmd/root.go index 24fc647c3..f8c1a0b06 100644 --- a/cmd/tink-worker/cmd/root.go +++ b/cmd/tink-worker/cmd/root.go @@ -6,7 +6,8 @@ import ( "time" dockercli "github.com/docker/docker/client" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" + "github.com/go-logr/zapr" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -14,6 +15,7 @@ import ( "github.com/tinkerbell/tink/cmd/tink-worker/worker" "github.com/tinkerbell/tink/internal/client" "github.com/tinkerbell/tink/internal/proto" + "go.uber.org/zap" ) const ( @@ -24,7 +26,13 @@ const ( ) // NewRootCommand creates a new Tink Worker Cobra root command. -func NewRootCommand(version string, logger log.Logger) *cobra.Command { +func NewRootCommand(version string) *cobra.Command { + zlog, err := zap.NewProduction() + if err != nil { + panic(err) + } + logger := zapr.NewLogger(zlog).WithName("github.com/tinkerbell/tink") + rootCmd := &cobra.Command{ Use: "tink-worker", Short: "Tink Worker", @@ -42,7 +50,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command { registry := viper.GetString("docker-registry") captureActionLogs := viper.GetBool("capture-action-logs") - logger.With("version", version).Info("starting") + logger.Info("starting", "version", version) conn, err := client.NewClientConn( viper.GetString("tinkerbell-grpc-authority"), @@ -99,7 +107,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command { must := func(err error) { if err != nil { - logger.Fatal(err) + logger.Error(err, "") } } @@ -117,7 +125,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command { // initViper initializes Viper configured to read in configuration files // (from various paths with content type specific filename extensions) and loads // environment variables. -func initViper(logger log.Logger, cmd *cobra.Command) error { +func initViper(logger logr.Logger, cmd *cobra.Command) error { viper.AutomaticEnv() viper.SetConfigName("tink-worker") viper.AddConfigPath("/etc/tinkerbell") @@ -127,11 +135,11 @@ func initViper(logger log.Logger, cmd *cobra.Command) error { // If a config file is found, read it in. if err := viper.ReadInConfig(); err != nil { if _, ok := err.(viper.ConfigFileNotFoundError); !ok { - logger.With("configFile", viper.ConfigFileUsed()).Error(err, "could not load config file") + logger.Error(err, "could not load config file", "configFile", viper.ConfigFileUsed()) return err } } else { - logger.With("configFile", viper.ConfigFileUsed()).Info("loaded config file") + logger.Info("loaded config file", "configFile", viper.ConfigFileUsed()) } cmd.Flags().VisitAll(func(f *pflag.Flag) { diff --git a/cmd/tink-worker/main.go b/cmd/tink-worker/main.go index e7a9627d4..881121f35 100644 --- a/cmd/tink-worker/main.go +++ b/cmd/tink-worker/main.go @@ -1,34 +1,17 @@ package main import ( - "context" "os" - "github.com/equinix-labs/otel-init-go/otelinit" - "github.com/packethost/pkg/log" "github.com/tinkerbell/tink/cmd/tink-worker/cmd" ) -const ( - serviceKey = "github.com/tinkerbell/tink" -) - // version is set at build time. var version = "devel" func main() { - logger, err := log.Init(serviceKey) - if err != nil { - panic(err) - } - - ctx, otelShutdown := otelinit.InitOpenTelemetry(context.Background(), "github.com/tinkerbell/tink") - - rootCmd := cmd.NewRootCommand(version, logger) + rootCmd := cmd.NewRootCommand(version) if err := rootCmd.Execute(); err != nil { os.Exit(1) } - - logger.Close() - otelShutdown(ctx) } diff --git a/cmd/tink-worker/worker/container_manager.go b/cmd/tink-worker/worker/container_manager.go index 2188997b2..13fea66d3 100644 --- a/cmd/tink-worker/worker/container_manager.go +++ b/cmd/tink-worker/worker/container_manager.go @@ -9,7 +9,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/tinkerbell/tink/internal/proto" ) @@ -28,24 +28,23 @@ type DockerClient interface { } type containerManager struct { - logger log.Logger + logger logr.Logger cli DockerClient registryDetails RegistryConnDetails } // getLogger is a helper function to get logging out of a context, or use the default logger. -func (m *containerManager) getLogger(ctx context.Context) *log.Logger { +func (m *containerManager) getLogger(ctx context.Context) logr.Logger { loggerIface := ctx.Value(loggingContextKey) if loggerIface == nil { - return &m.logger + return m.logger } - l, _ := loggerIface.(*log.Logger) - + l, _ := loggerIface.(logr.Logger) return l } // NewContainerManager returns a new container manager. -func NewContainerManager(logger log.Logger, cli DockerClient, registryDetails RegistryConnDetails) ContainerManager { +func NewContainerManager(logger logr.Logger, cli DockerClient, registryDetails RegistryConnDetails) ContainerManager { return &containerManager{logger, cli, registryDetails} } @@ -76,7 +75,7 @@ func (m *containerManager) CreateContainer(ctx context.Context, cmd []string, wf } hostConfig.Binds = append(hostConfig.Binds, action.GetVolumes()...) - l.With("command", cmd).Info("creating container") + l.Info("creating container", "command", cmd) name := makeValidContainerName(action.GetName()) resp, err := m.cli.ContainerCreate(ctx, config, hostConfig, nil, nil, name) if err != nil { @@ -94,7 +93,7 @@ func makeValidContainerName(name string) string { } func (m *containerManager) StartContainer(ctx context.Context, id string) error { - m.getLogger(ctx).With("containerID", id).Debug("starting container") + m.getLogger(ctx).Info("starting container", "containerID", id) return errors.Wrap(m.cli.ContainerStart(ctx, id, types.ContainerStartOptions{}), "DOCKER START") } @@ -133,10 +132,10 @@ func (m *containerManager) WaitForFailedContainer(ctx context.Context, id string } failedActionStatus <- proto.State_STATE_FAILED case err := <-errC: - l.Error(err) + l.Error(err, "") failedActionStatus <- proto.State_STATE_FAILED case <-ctx.Done(): - l.Error(ctx.Err()) + l.Error(ctx.Err(), "") failedActionStatus <- proto.State_STATE_TIMEOUT } } @@ -148,7 +147,7 @@ func (m *containerManager) RemoveContainer(ctx context.Context, id string) error RemoveLinks: false, RemoveVolumes: true, } - m.getLogger(ctx).With("containerID", id).Info("removing container") + m.getLogger(ctx).Info("removing container", "containerID", id) // send API call to remove the container return errors.Wrap(m.cli.ContainerRemove(ctx, id, opts), "DOCKER STOP") diff --git a/cmd/tink-worker/worker/container_manager_test.go b/cmd/tink-worker/worker/container_manager_test.go index 347e9a804..25af31394 100644 --- a/cmd/tink-worker/worker/container_manager_test.go +++ b/cmd/tink-worker/worker/container_manager_test.go @@ -10,9 +10,10 @@ import ( containertypes "github.com/docker/docker/api/types/container" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" + "github.com/go-logr/zapr" specs "github.com/opencontainers/image-spec/specs-go/v1" - "github.com/packethost/pkg/log" "github.com/tinkerbell/tink/internal/proto" + "go.uber.org/zap" ) type fakeDockerClient struct { @@ -131,7 +132,7 @@ func TestContainerManagerCreate(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) mgr := NewContainerManager(logger, newFakeDockerClient(tc.containerID, "", 0, 0, tc.clientErr, nil), RegistryConnDetails{Registry: tc.registry}) ctx := context.Background() @@ -177,7 +178,7 @@ func TestContainerManagerStart(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) mgr := NewContainerManager(logger, newFakeDockerClient(tc.containerID, "", 0, 0, tc.clientErr, nil), RegistryConnDetails{Registry: ""}) ctx := context.Background() @@ -249,7 +250,7 @@ func TestContainerManagerWait(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) mgr := NewContainerManager(logger, newFakeDockerClient(tc.containerID, "", time.Millisecond*20, tc.dockerResponse, tc.clientErr, tc.waitErr), RegistryConnDetails{Registry: ""}) ctx, cancel := context.WithTimeout(context.Background(), tc.contextTimeout) defer cancel() @@ -320,7 +321,7 @@ func TestContainerManagerWaitFailed(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) mgr := NewContainerManager(logger, newFakeDockerClient(tc.containerID, "", tc.waitTime, tc.dockerResponse, nil, tc.clientErr), RegistryConnDetails{Registry: ""}) ctx, cancel := context.WithTimeout(context.Background(), tc.contextTimeout) defer cancel() @@ -359,7 +360,7 @@ func TestContainerManagerRemove(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) mgr := NewContainerManager(logger, newFakeDockerClient(tc.containerID, "", 0, 0, tc.clientErr, nil), RegistryConnDetails{Registry: ""}) ctx := context.Background() diff --git a/cmd/tink-worker/worker/log_capturer.go b/cmd/tink-worker/worker/log_capturer.go index 6431d8d03..3b4d7ed1f 100644 --- a/cmd/tink-worker/worker/log_capturer.go +++ b/cmd/tink-worker/worker/log_capturer.go @@ -8,29 +8,28 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/client" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" ) // DockerLogCapturer is a LogCapturer that can stream docker container logs to an io.Writer. type DockerLogCapturer struct { dockerClient client.ContainerAPIClient - logger log.Logger + logger logr.Logger writer io.Writer } // getLogger is a helper function to get logging out of a context, or use the default logger. -func (l *DockerLogCapturer) getLogger(ctx context.Context) *log.Logger { +func (l *DockerLogCapturer) getLogger(ctx context.Context) logr.Logger { loggerIface := ctx.Value(loggingContextKey) if loggerIface == nil { - return &l.logger + return l.logger } - lg, _ := loggerIface.(*log.Logger) - + lg, _ := loggerIface.(logr.Logger) return lg } // NewDockerLogCapturer returns a LogCapturer that can stream container logs to a given writer. -func NewDockerLogCapturer(cli client.ContainerAPIClient, logger log.Logger, writer io.Writer) *DockerLogCapturer { +func NewDockerLogCapturer(cli client.ContainerAPIClient, logger logr.Logger, writer io.Writer) *DockerLogCapturer { return &DockerLogCapturer{ dockerClient: cli, logger: logger, @@ -47,7 +46,7 @@ func (l *DockerLogCapturer) CaptureLogs(ctx context.Context, id string) { Timestamps: false, }) if err != nil { - l.getLogger(ctx).Error(err, "failed to capture logs for container ", id) + l.getLogger(ctx).Error(err, "failed to capture logs for container ", "containerID", id) return } defer reader.Close() diff --git a/cmd/tink-worker/worker/log_capturer_test.go b/cmd/tink-worker/worker/log_capturer_test.go index e305af71f..0b37a6246 100644 --- a/cmd/tink-worker/worker/log_capturer_test.go +++ b/cmd/tink-worker/worker/log_capturer_test.go @@ -10,8 +10,10 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/client" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" + "github.com/go-logr/zapr" "github.com/pkg/errors" + "go.uber.org/zap" ) type fakeDockerLoggerClient struct { @@ -57,7 +59,7 @@ func TestLogCapturer(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) ctx := context.Background() clogger := NewDockerLogCapturer( newFakeDockerLoggerClient(tc.content, tc.wanterr), @@ -75,7 +77,7 @@ func TestLogCapturer(t *testing.T) { func TestLogCapturerContextLogger(t *testing.T) { cases := []struct { name string - logger func() *log.Logger + logger func() logr.Logger writer bytes.Buffer }{ { @@ -84,9 +86,8 @@ func TestLogCapturerContextLogger(t *testing.T) { }, { name: "with context logger", - logger: func() *log.Logger { - l := log.Test(t, "github.com/tinkerbell/tink/test") - return &l + logger: func() logr.Logger { + return zapr.NewLogger(zap.Must(zap.NewDevelopment())) }, writer: *bytes.NewBufferString(""), }, @@ -94,7 +95,7 @@ func TestLogCapturerContextLogger(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) ctx := context.Background() if tc.logger != nil { ctx = context.WithValue(ctx, loggingContextKey, tc.logger()) diff --git a/cmd/tink-worker/worker/registry.go b/cmd/tink-worker/worker/registry.go index 5303f99e3..02609a80b 100644 --- a/cmd/tink-worker/worker/registry.go +++ b/cmd/tink-worker/worker/registry.go @@ -49,7 +49,7 @@ func (m *containerManager) PullImage(ctx context.Context, image string) error { } defer func() { if err := out.Close(); err != nil { - l.Error(err) + l.Error(err, "") } }() fd := json.NewDecoder(out) diff --git a/cmd/tink-worker/worker/registry_test.go b/cmd/tink-worker/worker/registry_test.go index 06efd9534..1ce8a3f92 100644 --- a/cmd/tink-worker/worker/registry_test.go +++ b/cmd/tink-worker/worker/registry_test.go @@ -8,7 +8,8 @@ import ( "testing" "github.com/docker/docker/api/types" - "github.com/packethost/pkg/log" + "github.com/go-logr/zapr" + "go.uber.org/zap" ) func (c *fakeDockerClient) ImagePull(context.Context, string, types.ImagePullOptions) (io.ReadCloser, error) { @@ -49,7 +50,7 @@ func TestContainerManagerPullImage(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - logger := log.Test(t, "github.com/tinkerbell/tink") + logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) mgr := NewContainerManager(logger, newFakeDockerClient("", tc.responseContent, 0, 0, tc.clientErr, nil), tc.registry) ctx := context.Background() diff --git a/cmd/tink-worker/worker/worker.go b/cmd/tink-worker/worker/worker.go index 4daef36d6..d578e1e71 100644 --- a/cmd/tink-worker/worker/worker.go +++ b/cmd/tink-worker/worker/worker.go @@ -7,7 +7,7 @@ import ( "strconv" "time" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/tinkerbell/tink/internal/proto" ) @@ -95,7 +95,7 @@ type Worker struct { logCapturer LogCapturer containerManager ContainerManager tinkClient proto.WorkflowServiceClient - logger log.Logger + logger logr.Logger dataDir string maxSize int64 @@ -113,7 +113,7 @@ func NewWorker( tinkClient proto.WorkflowServiceClient, containerManager ContainerManager, logCapturer LogCapturer, - logger log.Logger, + logger logr.Logger, opts ...Option, ) *Worker { w := &Worker{ @@ -137,19 +137,18 @@ func NewWorker( } // getLogger is a helper function to get logging out of a context, or use the default logger. -func (w Worker) getLogger(ctx context.Context) *log.Logger { +func (w Worker) getLogger(ctx context.Context) logr.Logger { loggerIface := ctx.Value(loggingContextKey) if loggerIface == nil { - return &w.logger + return w.logger } - l, _ := loggerIface.(*log.Logger) - + l, _ := loggerIface.(logr.Logger) return l } // execute executes a workflow action, optionally capturing logs. func (w *Worker) execute(ctx context.Context, wfID string, action *proto.WorkflowAction) (proto.State, error) { - l := w.getLogger(ctx).With("workflowID", wfID, "workerID", action.GetWorkerId(), "actionName", action.GetName(), "actionImage", action.GetImage()) + l := w.getLogger(ctx).WithValues("workflowID", wfID, "workerID", action.GetWorkerId(), "actionName", action.GetName(), "actionImage", action.GetImage()) if err := w.containerManager.PullImage(ctx, action.GetImage()); err != nil { return proto.State_STATE_RUNNING, errors.Wrap(err, "pull image") @@ -160,7 +159,7 @@ func (w *Worker) execute(ctx context.Context, wfID string, action *proto.Workflo return proto.State_STATE_RUNNING, errors.Wrap(err, "create container") } - l.With("containerID", id, "command", action.Command).Info("container created") + l.Info("container created", "containerID", id, "command", action.Command) var timeCtx context.Context var cancel context.CancelFunc @@ -182,16 +181,16 @@ func (w *Worker) execute(ctx context.Context, wfID string, action *proto.Workflo } st, err := w.containerManager.WaitForContainer(timeCtx, id) - l.With("status", st.String()).Info("wait container completed") + l.Info("wait container completed", "status", st.String()) // If we've made it this far, the container has successfully completed. // Everything after this is just cleanup. defer func() { if err := w.containerManager.RemoveContainer(ctx, id); err != nil { - l.With("containerID", id).Error(err) + l.Error(err, "remove container", "containerID", id) } - l.With("status", st.String()).Info("container removed") + l.Info("container removed", "status", st.String()) }() if err != nil { @@ -199,24 +198,24 @@ func (w *Worker) execute(ctx context.Context, wfID string, action *proto.Workflo } if st == proto.State_STATE_SUCCESS { - l.With("status", st).Info("action container exited with success") + l.Info("action container exited with success", "status", st) return st, nil } if st == proto.State_STATE_TIMEOUT && action.OnTimeout != nil { rst := w.executeReaction(ctx, st.String(), action.OnTimeout, wfID, action) - l.With("status", rst).Info("action timeout") + l.Info("action timeout", "status", rst) } else if action.OnFailure != nil { rst := w.executeReaction(ctx, st.String(), action.OnFailure, wfID, action) - l.With("status", rst).Info("action failed") + l.Info("action failed", "status", rst) } l.Info(infoWaitFinished) if err != nil { - l.Error(errors.Wrap(err, errFailedToWait)) + l.Error(err, errFailedToWait) } - l.With("status", st).Info("action container exited") + l.Info("action container exited", "status", st) return st, nil } @@ -225,9 +224,9 @@ func (w *Worker) executeReaction(ctx context.Context, reaction string, cmd []str l := w.getLogger(ctx) id, err := w.containerManager.CreateContainer(ctx, cmd, wfID, action, w.captureLogs, w.createPrivileged) if err != nil { - l.Error(errors.Wrap(err, errFailedToRunCmd)) + l.Error(err, errFailedToRunCmd) } - l.With("containerID", id, "actionStatus", reaction, "command", cmd).Info("container created") + l.Info("container created", "containerID", id, "actionStatus", reaction, "command", cmd) if w.captureLogs { go w.logCapturer.CaptureLogs(ctx, id) @@ -238,7 +237,7 @@ func (w *Worker) executeReaction(ctx context.Context, reaction string, cmd []str go w.containerManager.WaitForFailedContainer(ctx, id, st) err = w.containerManager.StartContainer(ctx, id) if err != nil { - l.Error(errors.Wrap(err, errFailedToRunCmd)) + l.Error(err, errFailedToRunCmd) } return <-st @@ -246,7 +245,7 @@ func (w *Worker) executeReaction(ctx context.Context, reaction string, cmd []str // ProcessWorkflowActions gets all Workflow contexts and processes their actions. func (w *Worker) ProcessWorkflowActions(ctx context.Context) error { - l := w.logger.With("workerID", w.workerID) + l := w.logger.WithValues("workerID", w.workerID) l.Info("starting to process workflow actions") for { @@ -257,7 +256,7 @@ func (w *Worker) ProcessWorkflowActions(ctx context.Context) error { } res, err := w.tinkClient.GetWorkflowContexts(ctx, &proto.WorkflowContextRequest{WorkerId: w.workerID}) if err != nil { - l.Error(errors.Wrap(err, errGetWfContext)) + l.Error(err, errGetWfContext) <-time.After(w.retryInterval) continue } @@ -270,18 +269,18 @@ func (w *Worker) ProcessWorkflowActions(ctx context.Context) error { wfContext, err := res.Recv() if err != nil || wfContext == nil { if !errors.Is(err, io.EOF) { - l.Info(err) + l.Info(err.Error()) } <-time.After(w.retryInterval) break } wfID := wfContext.GetWorkflowId() - l = l.With("workflowID", wfID) - ctx := context.WithValue(ctx, loggingContextKey, &l) + l = l.WithValues("workflowID", wfID) + ctx := context.WithValue(ctx, loggingContextKey, l) actions, err := w.tinkClient.GetWorkflowActions(ctx, &proto.WorkflowActionsRequest{WorkflowId: wfID}) if err != nil { - l.Error(errors.Wrap(err, errGetWfActions)) + l.Error(err, errGetWfActions) continue } @@ -317,11 +316,11 @@ func (w *Worker) ProcessWorkflowActions(ctx context.Context) error { for turn { l.Info("starting action") action := actions.GetActionList()[actionIndex] - l := l.With( + l := l.WithValues( "actionName", action.GetName(), "taskName", action.GetTaskName(), ) - ctx := context.WithValue(ctx, loggingContextKey, &l) + ctx := context.WithValue(ctx, loggingContextKey, l) if wfContext.GetCurrentActionState() != proto.State_STATE_RUNNING { actionStatus := &proto.WorkflowActionStatus{ WorkflowId: wfID, @@ -333,7 +332,7 @@ func (w *Worker) ProcessWorkflowActions(ctx context.Context) error { WorkerId: action.GetWorkerId(), } w.reportActionStatus(ctx, l, actionStatus) - l.With("status", actionStatus.ActionStatus, "duration", strconv.FormatInt(actionStatus.Seconds, 10)).Info("sent action status") + l.Info("sent action status", "status", actionStatus.ActionStatus, "duration", strconv.FormatInt(actionStatus.Seconds, 10)) } // start executing the action @@ -355,8 +354,8 @@ func (w *Worker) ProcessWorkflowActions(ctx context.Context) error { } else { actionStatus.ActionStatus = proto.State_STATE_FAILED } - l = l.With("actionStatus", actionStatus.ActionStatus.String()) - l.Error(err) + l = l.WithValues("actionStatus", actionStatus.ActionStatus.String()) + l.Error(err, "execute workflow") w.reportActionStatus(ctx, l, actionStatus) break } @@ -374,7 +373,7 @@ func (w *Worker) ProcessWorkflowActions(ctx context.Context) error { nextAction := actions.GetActionList()[actionIndex+1] if nextAction.GetWorkerId() != w.workerID { - l.Debug(fmt.Sprintf(msgTurn, nextAction.GetWorkerId())) + l.Info(fmt.Sprintf(msgTurn, nextAction.GetWorkerId())) turn = false } else { actionIndex++ @@ -391,12 +390,12 @@ func isLastAction(wfContext *proto.WorkflowContext, actions *proto.WorkflowActio } // reportActionStatus reports the status of an action to the Tinkerbell server and retries forever on error. -func (w *Worker) reportActionStatus(ctx context.Context, l log.Logger, actionStatus *proto.WorkflowActionStatus) { +func (w *Worker) reportActionStatus(ctx context.Context, l logr.Logger, actionStatus *proto.WorkflowActionStatus) { for { l.Info("reporting Action Status") _, err := w.tinkClient.ReportActionStatus(ctx, actionStatus) if err != nil { - l.Error(errors.Wrap(err, errReportActionStatus)) + l.Error(err, errReportActionStatus) <-time.After(w.retryInterval) continue diff --git a/cmd/virtual-worker/cmd/root.go b/cmd/virtual-worker/cmd/root.go index 24ede6a85..c6dfa706b 100644 --- a/cmd/virtual-worker/cmd/root.go +++ b/cmd/virtual-worker/cmd/root.go @@ -4,7 +4,8 @@ import ( "strings" "time" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" + "github.com/go-logr/zapr" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -13,6 +14,7 @@ import ( "github.com/tinkerbell/tink/cmd/virtual-worker/worker" "github.com/tinkerbell/tink/internal/client" "github.com/tinkerbell/tink/internal/proto" + "go.uber.org/zap" ) const ( @@ -22,7 +24,13 @@ const ( ) // NewRootCommand creates a new Virtual Worker Cobra root command. -func NewRootCommand(version string, logger log.Logger) *cobra.Command { +func NewRootCommand(version string) *cobra.Command { + zlog, err := zap.NewProduction() + if err != nil { + panic(err) + } + logger := zapr.NewLogger(zlog).WithName("github.com/tinkerbell/tink") + rootCmd := &cobra.Command{ Use: "virtual-worker", Short: "Virtual Tink Worker", @@ -38,7 +46,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command { sleepMin := viper.GetDuration("sleep-min") sleepJitter := viper.GetDuration("sleep-jitter") - logger.With("version", version).Info("starting") + logger.Info("starting", "version", version) conn, err := client.NewClientConn( viper.GetString("tinkerbell-grpc-authority"), @@ -80,7 +88,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command { must := func(err error) { if err != nil { - logger.Fatal(err) + logger.Error(err, "") } } @@ -95,7 +103,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command { // createViper creates a Viper object configured to read in configuration files // (from various paths with content type specific filename extensions) and loads // environment variables. -func createViper(logger log.Logger, cmd *cobra.Command) error { +func createViper(logger logr.Logger, cmd *cobra.Command) error { viper.AutomaticEnv() viper.SetConfigName("virtual-worker") viper.AddConfigPath("/etc/tinkerbell") @@ -105,12 +113,12 @@ func createViper(logger log.Logger, cmd *cobra.Command) error { // If a config file is found, read it in. if err := viper.ReadInConfig(); err != nil { if _, ok := err.(viper.ConfigFileNotFoundError); !ok { - logger.With("configFile", viper.ConfigFileUsed()).Error(err, "could not load config file") + logger.Error(err, "could not load config file", "configFile", viper.ConfigFileUsed()) return err } logger.Info("no config file found") } else { - logger.With("configFile", viper.ConfigFileUsed()).Info("loaded config file") + logger.Info("loaded config file", "configFile", viper.ConfigFileUsed()) } cmd.Flags().VisitAll(func(f *pflag.Flag) { diff --git a/cmd/virtual-worker/main.go b/cmd/virtual-worker/main.go index 720699f3c..6f3293aac 100644 --- a/cmd/virtual-worker/main.go +++ b/cmd/virtual-worker/main.go @@ -3,25 +3,15 @@ package main import ( "os" - "github.com/packethost/pkg/log" "github.com/tinkerbell/tink/cmd/virtual-worker/cmd" ) -const ( - serviceKey = "github.com/tinkerbell/tink" -) - // version is set at build time. var version = "devel" func main() { - logger, err := log.Init(serviceKey) - if err != nil { - panic(err) - } - rootCmd := cmd.NewRootCommand(version, logger) + rootCmd := cmd.NewRootCommand(version) if err := rootCmd.Execute(); err != nil { os.Exit(1) } - logger.Close() } diff --git a/cmd/virtual-worker/worker/container_manager.go b/cmd/virtual-worker/worker/container_manager.go index b0ed0f832..4a9b8fa10 100644 --- a/cmd/virtual-worker/worker/container_manager.go +++ b/cmd/virtual-worker/worker/container_manager.go @@ -5,7 +5,7 @@ import ( "math/rand" "time" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" "github.com/tinkerbell/tink/cmd/tink-worker/worker" "github.com/tinkerbell/tink/internal/proto" ) @@ -26,7 +26,7 @@ type fakeManager struct { sleepJitter time.Duration r *rand.Rand - logger log.Logger + logger logr.Logger } func (m *fakeManager) sleep() { @@ -35,7 +35,7 @@ func (m *fakeManager) sleep() { } // NewFakeContainerManager returns a fake worker.ContainerManager that will sleep for Docker API calls. -func NewFakeContainerManager(l log.Logger, sleepMinimum, sleepJitter time.Duration) worker.ContainerManager { +func NewFakeContainerManager(l logr.Logger, sleepMinimum, sleepJitter time.Duration) worker.ContainerManager { if sleepMinimum <= 0 { sleepMinimum = 1 } @@ -52,35 +52,35 @@ func NewFakeContainerManager(l log.Logger, sleepMinimum, sleepJitter time.Durati } func (m *fakeManager) CreateContainer(_ context.Context, cmd []string, _ string, _ *proto.WorkflowAction, _, _ bool) (string, error) { - m.logger.With("command", cmd).Info("creating container") + m.logger.Info("creating container", "command", cmd) return getRandHexStr(m.r, 64), nil } func (m *fakeManager) StartContainer(_ context.Context, id string) error { - m.logger.With("containerID", id).Debug("starting container") + m.logger.Info("starting container", "containerID", id) return nil } func (m *fakeManager) WaitForContainer(_ context.Context, id string) (proto.State, error) { - m.logger.With("containerID", id).Info("waiting for container") + m.logger.Info("waiting for container", "containerID", id) m.sleep() return proto.State_STATE_SUCCESS, nil } func (m *fakeManager) WaitForFailedContainer(_ context.Context, id string, failedActionStatus chan proto.State) { - m.logger.With("containerID", id).Info("waiting for container") + m.logger.Info("waiting for container", "containerID", id) m.sleep() failedActionStatus <- proto.State_STATE_SUCCESS } func (m *fakeManager) RemoveContainer(_ context.Context, id string) error { - m.logger.With("containerID", id).Info("removing container") + m.logger.Info("removing container", "containerID", id) return nil } func (m *fakeManager) PullImage(_ context.Context, image string) error { - m.logger.With("image", image).Info("pulling image") + m.logger.Info("pulling image", "image", image) m.sleep() return nil diff --git a/go.mod b/go.mod index 1a53f5b3f..88b08eaab 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,17 @@ require ( sigs.k8s.io/yaml v1.3.0 ) +require ( + golang.org/x/mod v0.9.0 // indirect + golang.org/x/net v0.8.0 // indirect + golang.org/x/oauth2 v0.4.0 // indirect + golang.org/x/sys v0.6.0 // indirect + golang.org/x/term v0.6.0 // indirect + golang.org/x/text v0.8.0 // indirect + golang.org/x/time v0.1.0 // indirect + golang.org/x/tools v0.7.0 // indirect +) + require ( github.com/Microsoft/go-winio v0.6.0 // indirect github.com/benbjohnson/clock v1.3.0 // indirect @@ -73,7 +84,6 @@ require ( github.com/prometheus/common v0.37.0 // indirect github.com/prometheus/procfs v0.8.0 // indirect github.com/rogpeppe/go-internal v1.9.0 // indirect - github.com/rollbar/rollbar-go v1.0.2 // indirect github.com/sirupsen/logrus v1.9.0 // indirect github.com/spf13/afero v1.9.3 // indirect github.com/spf13/cast v1.5.0 // indirect @@ -89,14 +99,6 @@ require ( go.opentelemetry.io/proto/otlp v0.12.0 // indirect go.uber.org/atomic v1.10.0 // indirect go.uber.org/multierr v1.9.0 // indirect - golang.org/x/mod v0.9.0 // indirect - golang.org/x/net v0.8.0 // indirect - golang.org/x/oauth2 v0.4.0 // indirect - golang.org/x/sys v0.6.0 // indirect - golang.org/x/term v0.6.0 // indirect - golang.org/x/text v0.8.0 // indirect - golang.org/x/time v0.1.0 // indirect - golang.org/x/tools v0.7.0 // indirect gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f // indirect diff --git a/go.sum b/go.sum index 47dea458d..9f74ee1c4 100644 --- a/go.sum +++ b/go.sum @@ -768,7 +768,6 @@ github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6L github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= -github.com/rollbar/rollbar-go v1.0.2 h1:uA3+z0jq6ka9WUUt9VX/xuiQZXZyWRoeKvkhVvLO9Jc= github.com/rollbar/rollbar-go v1.0.2/go.mod h1:AcFs5f0I+c71bpHlXNNDbOWJiKwjFDtISeXco0L5PKQ= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= diff --git a/internal/e2e/e2e_test.go b/internal/e2e/e2e_test.go index aa276cc1b..b95f3a19a 100644 --- a/internal/e2e/e2e_test.go +++ b/internal/e2e/e2e_test.go @@ -99,7 +99,7 @@ var _ = Describe("Tink API", func() { worker.WithDataDir("./worker"), worker.WithMaxFileSize(1<<10), worker.WithRetries(time.Millisecond*500, 3)) - logger.With("workerID", workerID).Info("Created worker") + logger.Info("Created worker", "workerID", workerID) errChan := make(chan error) workerCtx, cancel := context.WithTimeout(ctx, time.Second*8) diff --git a/internal/e2e/tink_suite_test.go b/internal/e2e/tink_suite_test.go index a73f466d6..fd62bda50 100644 --- a/internal/e2e/tink_suite_test.go +++ b/internal/e2e/tink_suite_test.go @@ -7,10 +7,10 @@ import ( "testing" "time" + "github.com/go-logr/logr" "github.com/go-logr/zapr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/packethost/pkg/log" "github.com/tinkerbell/tink/api/v1alpha1" "github.com/tinkerbell/tink/internal/controller" "github.com/tinkerbell/tink/internal/grpcserver" @@ -28,7 +28,7 @@ var ( ctx context.Context cancel context.CancelFunc serverAddr string - logger log.Logger + logger logr.Logger ) func TestTests(t *testing.T) { @@ -40,8 +40,7 @@ var _ = BeforeSuite(func() { ctx, cancel = context.WithCancel(context.TODO()) var err error - logger, err = log.Init("github.com/tinkerbell/tink/tests") - Expect(err).NotTo(HaveOccurred()) + logger = zapr.NewLogger(zap.Must(zap.NewDevelopment())) // Installs CRDs into cluster By("bootstrapping test environment") @@ -55,7 +54,7 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(cfg).NotTo(BeNil()) cfg.Timeout = time.Second * 5 // Graceful shutdown of testenv for only 5s - logger.With("host", cfg.Host).Info("started test environment") + logger.Info("started test environment", "host", cfg.Host) // Add tink API to the client scheme err = v1alpha1.AddToScheme(scheme.Scheme) @@ -78,14 +77,11 @@ var _ = BeforeSuite(func() { errCh, ) Expect(err).NotTo(HaveOccurred()) - logger.Info("HTTP server: ", fmt.Sprintf("%+v", serverAddr)) + logger.Info(fmt.Sprintf("HTTP server: %v", serverAddr)) // Start the controller - zapLogger, err := zap.NewDevelopment() - Expect(err).To(Succeed()) - options := ctrl.Options{ - Logger: zapr.NewLogger(zapLogger), + Logger: logger, } manager, err := controller.NewManager(cfg, options) diff --git a/internal/httpserver/http_server.go b/internal/httpserver/http_server.go index 2677986bf..3fcdf915b 100644 --- a/internal/httpserver/http_server.go +++ b/internal/httpserver/http_server.go @@ -7,7 +7,7 @@ import ( "runtime" "time" - "github.com/packethost/pkg/log" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus/promhttp" ) @@ -15,11 +15,11 @@ import ( var ( gitRev = "unknown" startTime = time.Now() - logger log.Logger + logger logr.Logger ) // SetupHTTP setup and return an HTTP server. -func SetupHTTP(ctx context.Context, logger log.Logger, authority string, errCh chan<- error) { +func SetupHTTP(ctx context.Context, logger logr.Logger, authority string, errCh chan<- error) { http.Handle("/metrics", promhttp.Handler()) http.HandleFunc("/version", getGitRevJSONHandler()) http.HandleFunc("/healthz", healthCheckHandler) @@ -38,7 +38,7 @@ func SetupHTTP(ctx context.Context, logger log.Logger, authority string, errCh c go func() { <-ctx.Done() if err := srv.Shutdown(context.Background()); err != nil { - logger.Error(err) + logger.Error(err, "shutting down http server") } }() } @@ -74,7 +74,7 @@ func getGitRevJSONHandler() http.HandlerFunc { b, err := json.Marshal(&res) if err != nil { err = errors.Wrap(err, "could not marshal version json") - logger.Error(err) + logger.Error(err, "") panic(err) } diff --git a/internal/server/kubernetes_api.go b/internal/server/kubernetes_api.go index 8c9ff9f78..6ec61a2c0 100644 --- a/internal/server/kubernetes_api.go +++ b/internal/server/kubernetes_api.go @@ -5,8 +5,8 @@ import ( "fmt" "time" + "github.com/go-logr/logr" "github.com/go-logr/zapr" - "github.com/packethost/pkg/log" "github.com/tinkerbell/tink/api/v1alpha1" "github.com/tinkerbell/tink/internal/controller" "github.com/tinkerbell/tink/internal/proto" @@ -24,7 +24,7 @@ import ( // +kubebuilder:rbac:groups=tinkerbell.org,resources=workflows;workflows/status,verbs=get;list;watch;update;patch // NewKubeBackedServer returns a server that implements the Workflow server interface for a given kubeconfig. -func NewKubeBackedServer(logger log.Logger, kubeconfig, apiserver, namespace string) (*KubernetesBackedServer, error) { +func NewKubeBackedServer(logger logr.Logger, kubeconfig, apiserver, namespace string) (*KubernetesBackedServer, error) { ccfg := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( &clientcmd.ClientConfigLoadingRules{ExplicitPath: kubeconfig}, &clientcmd.ConfigOverrides{ @@ -56,7 +56,7 @@ func NewKubeBackedServer(logger log.Logger, kubeconfig, apiserver, namespace str // NewKubeBackedServerFromREST returns a server that implements the Workflow // server interface with the given Kubernetes rest client and namespace. -func NewKubeBackedServerFromREST(logger log.Logger, config *rest.Config, namespace string) (*KubernetesBackedServer, error) { +func NewKubeBackedServerFromREST(logger logr.Logger, config *rest.Config, namespace string) (*KubernetesBackedServer, error) { clstr, err := cluster.New(config, func(opts *cluster.Options) { opts.Scheme = controller.DefaultScheme() opts.Logger = zapr.NewLogger(zap.NewNop()) @@ -93,7 +93,7 @@ func NewKubeBackedServerFromREST(logger log.Logger, config *rest.Config, namespa // KubernetesBackedServer is a server that implements a workflow API. type KubernetesBackedServer struct { - logger log.Logger + logger logr.Logger ClientFunc func() client.Client namespace string diff --git a/internal/server/kubernetes_api_test.go b/internal/server/kubernetes_api_test.go index fdab4a07e..1a50836bb 100644 --- a/internal/server/kubernetes_api_test.go +++ b/internal/server/kubernetes_api_test.go @@ -5,11 +5,12 @@ import ( "testing" "time" + "github.com/go-logr/zapr" "github.com/google/go-cmp/cmp" - "github.com/packethost/pkg/log" "github.com/tinkerbell/tink/api/v1alpha1" "github.com/tinkerbell/tink/internal/proto" "github.com/tinkerbell/tink/internal/testtime" + "go.uber.org/zap" ) var TestTime = testtime.NewFrozenTimeUnix(1637361793) @@ -418,7 +419,7 @@ func TestModifyWorkflowState(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { server := &KubernetesBackedServer{ - logger: log.Test(t, "TestModifyWorkflowState"), + logger: zapr.NewLogger(zap.Must(zap.NewDevelopment())), ClientFunc: nil, namespace: "default", nowFunc: TestTime.Now, diff --git a/internal/server/kubernetes_api_workflow.go b/internal/server/kubernetes_api_workflow.go index f3fa7ba34..17e7fdff1 100644 --- a/internal/server/kubernetes_api_workflow.go +++ b/internal/server/kubernetes_api_workflow.go @@ -56,7 +56,7 @@ func (s *KubernetesBackedServer) getWorkflowByName(ctx context.Context, workflow wflw := &v1alpha1.Workflow{} err := s.ClientFunc().Get(ctx, types.NamespacedName{Name: workflowID, Namespace: namespace}, wflw) if err != nil { - s.logger.With("workflow", workflowID).Error(err) + s.logger.Error(err, "get client", "workflow", workflowID) return nil, err } return wflw, nil @@ -184,11 +184,11 @@ func (s *KubernetesBackedServer) ReportActionStatus(ctx context.Context, req *pr return nil, err } wfID := req.GetWorkflowId() - l := s.logger.With("actionName", req.GetActionName(), "status", req.GetActionStatus(), "workflowID", req.GetWorkflowId(), "taskName", req.GetTaskName(), "worker", req.WorkerId) + l := s.logger.WithValues("actionName", req.GetActionName(), "status", req.GetActionStatus(), "workflowID", req.GetWorkflowId(), "taskName", req.GetTaskName(), "worker", req.WorkerId) wf, err := s.getWorkflowByName(ctx, wfID, s.namespace) if err != nil { - l.Error(err) + l.Error(err, "get workflow") return nil, status.Errorf(codes.InvalidArgument, errInvalidWorkflowID) } if req.GetTaskName() != wf.GetCurrentTask() { @@ -201,13 +201,13 @@ func (s *KubernetesBackedServer) ReportActionStatus(ctx context.Context, req *pr wfContext := getWorkflowContextForRequest(req, wf) err = s.modifyWorkflowState(wf, wfContext) if err != nil { - l.Error(err) + l.Error(err, "modify workflow state") return nil, status.Errorf(codes.InvalidArgument, errInvalidWorkflowID) } l.Info("updating workflow in Kubernetes") err = s.ClientFunc().Status().Update(ctx, wf) if err != nil { - l.Error(err) + l.Error(err, "applying update to workflow") return nil, status.Errorf(codes.InvalidArgument, errInvalidWorkflowID) } return &proto.Empty{}, nil From 35c8378ec73b1e52bd2b158998a9d6fb59b897ba Mon Sep 17 00:00:00 2001 From: Chris Doherty Date: Sat, 25 Mar 2023 21:34:02 -0500 Subject: [PATCH 2/2] Remove packethost env config dependency Signed-off-by: Chris Doherty --- cmd/tink-server/main.go | 10 +++++++--- go.mod | 1 - go.sum | 9 --------- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/cmd/tink-server/main.go b/cmd/tink-server/main.go index 1331a4dfd..a9cae0bdc 100644 --- a/cmd/tink-server/main.go +++ b/cmd/tink-server/main.go @@ -11,7 +11,6 @@ import ( "github.com/equinix-labs/otel-init-go/otelinit" "github.com/go-logr/logr" "github.com/go-logr/zapr" - "github.com/packethost/pkg/env" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -52,8 +51,13 @@ func (c *Config) AddFlags(fs *pflag.FlagSet) { } func (c *Config) PopulateFromLegacyEnvVar() { - c.GRPCAuthority = env.Get("TINKERBELL_GRPC_AUTHORITY", c.GRPCAuthority) - c.HTTPAuthority = env.Get("TINKERBELL_HTTP_AUTHORITY", c.HTTPAuthority) + if v, ok := os.LookupEnv("TINKERBELL_GRPC_AUTHORITY"); ok { + c.GRPCAuthority = v + } + + if v, ok := os.LookupEnv("TINKERBELL_HTTP_AUTHORITY"); ok { + c.HTTPAuthority = v + } } func main() { diff --git a/go.mod b/go.mod index 88b08eaab..c31fa09d0 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,6 @@ require ( github.com/onsi/ginkgo/v2 v2.9.2 github.com/onsi/gomega v1.27.5 github.com/opencontainers/image-spec v1.1.0-rc2 - github.com/packethost/pkg v0.0.0-20200903155310-0433e0605550 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.14.0 github.com/spf13/cobra v1.6.1 diff --git a/go.sum b/go.sum index 9f74ee1c4..736b0d0e4 100644 --- a/go.sum +++ b/go.sum @@ -609,7 +609,6 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= -github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= @@ -703,8 +702,6 @@ github.com/opencontainers/image-spec v1.1.0-rc2/go.mod h1:3OVijpioIKYWTqjiG0zfF6 github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4= github.com/openzipkin/zipkin-go v0.3.0/go.mod h1:4c3sLeE8xjNqehmF5RpAFLPLJxXscc0R4l6Zg0P1tTQ= -github.com/packethost/pkg v0.0.0-20200903155310-0433e0605550 h1:/ojL7LAVjyH1MY+db0+j6rcWU3UWWpzHksYFsHWs9vQ= -github.com/packethost/pkg v0.0.0-20200903155310-0433e0605550/go.mod h1:GSv7cTtIjns4yc0pyajaM1RE/KE4djJONoblFIRDrxA= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pelletier/go-toml v1.9.3/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c= @@ -741,7 +738,6 @@ github.com/prometheus/client_model v0.3.0/go.mod h1:LDGWKZIo7rky3hgvBe+caln+Dr3d github.com/prometheus/common v0.0.0-20181113130724-41aa239b4cce/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro= github.com/prometheus/common v0.4.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= github.com/prometheus/common v0.4.1/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= -github.com/prometheus/common v0.6.0/go.mod h1:eBmuwkDJBwy6iBfxCBob6t6dR6ENT/y+J+Zk0j9GMYc= github.com/prometheus/common v0.10.0/go.mod h1:Tlit/dnDKsSWFlCLTWaA1cyBgKHSMdTB80sz/V91rCo= github.com/prometheus/common v0.26.0/go.mod h1:M7rCNAaPfAosfx8veZJCuw84e35h3Cfd9VFqTh1DIvc= github.com/prometheus/common v0.28.0/go.mod h1:vu+V0TpY+O6vW9J44gczi3Ap/oXXR10b+M/gUGO4Hls= @@ -751,7 +747,6 @@ github.com/prometheus/common v0.37.0/go.mod h1:phzohg0JFMnBEFGxTDbfu3QyL5GI8gTQJ github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= -github.com/prometheus/procfs v0.0.3/go.mod h1:4A/X28fw3Fc593LaREMrKMqOKvUAntwMDaekg4FpcdQ= github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU= github.com/prometheus/procfs v0.2.0/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU= github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA= @@ -768,7 +763,6 @@ github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6L github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= -github.com/rollbar/rollbar-go v1.0.2/go.mod h1:AcFs5f0I+c71bpHlXNNDbOWJiKwjFDtISeXco0L5PKQ= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= @@ -1083,7 +1077,6 @@ golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190606165138-5da285871e9c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190616124812-15dcb6c0061f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20190712062909-fae7ac547cb7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -1255,7 +1248,6 @@ google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoA google.golang.org/genproto v0.0.0-20190307195333-5fe7a883aa19/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= google.golang.org/genproto v0.0.0-20190425155659-357c62f0e4bb/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= google.golang.org/genproto v0.0.0-20190502173448-54afdca5d873/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= -google.golang.org/genproto v0.0.0-20190708153700-3bdd9d9f5532/go.mod h1:z3L6/3dTEVtUr6QSP8miRzeRqwQOioJ9I66odjN4I7s= google.golang.org/genproto v0.0.0-20190801165951-fa694d86fc64/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc= google.golang.org/genproto v0.0.0-20190911173649-1774047e7e51/go.mod h1:IbNlFCBrqXvoKpeg0TB2l7cyZUmoaFKYIwrEpbDKLA8= @@ -1328,7 +1320,6 @@ google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZi google.golang.org/grpc v1.20.0/go.mod h1:chYK+tFQF0nDUGJgXMSgLCQk3phJEuONr2DCgLDdAQM= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= -google.golang.org/grpc v1.22.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= google.golang.org/grpc v1.26.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=