Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes inline os.Stdout as log output #5550

Merged
merged 3 commits into from
Mar 24, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions internal/cmd/certgen.go
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@
"context"
"errors"
"fmt"
"io"
"path"

"github.com/spf13/cobra"
@@ -36,7 +37,7 @@
Use: "certgen",
Short: "Generate Control Plane Certificates",
RunE: func(cmd *cobra.Command, args []string) error {
return certGen(cmd.Context(), local)
return certGen(cmd.Context(), cmd.OutOrStdout(), local)

Check warning on line 40 in internal/cmd/certgen.go

Codecov / codecov/patch

internal/cmd/certgen.go#L40

Added line #L40 was not covered by tests
},
}

@@ -48,8 +49,8 @@
}

// certGen generates control plane certificates.
func certGen(ctx context.Context, local bool) error {
cfg, err := config.New()
func certGen(ctx context.Context, logOut io.Writer, local bool) error {
cfg, err := config.New(logOut)

Check warning on line 53 in internal/cmd/certgen.go

Codecov / codecov/patch

internal/cmd/certgen.go#L52-L53

Added lines #L52 - L53 were not covered by tests
if err != nil {
return err
}
13 changes: 6 additions & 7 deletions internal/cmd/egctl/status.go
Original file line number Diff line number Diff line change
@@ -9,7 +9,6 @@
"context"
"fmt"
"io"
"os"
"reflect"
"strconv"
"strings"
@@ -93,27 +92,27 @@
switch strings.ToLower(resourceType) {
case "all":
for _, rt := range supportedAllTypes {
if err = runStatus(ctx, k8sClient, rt, namespace, quiet, verbose, allNamespaces, true, true); err != nil {
if err = runStatus(ctx, cmd.OutOrStdout(), k8sClient, rt, namespace, quiet, verbose, allNamespaces, true, true); err != nil {

Check warning on line 95 in internal/cmd/egctl/status.go

Codecov / codecov/patch

internal/cmd/egctl/status.go#L95

Added line #L95 was not covered by tests
return err
}
}
return nil
case "xroute":
for _, rt := range supportedXRouteTypes {
if err = runStatus(ctx, k8sClient, rt, namespace, quiet, verbose, allNamespaces, true, true); err != nil {
if err = runStatus(ctx, cmd.OutOrStdout(), k8sClient, rt, namespace, quiet, verbose, allNamespaces, true, true); err != nil {

Check warning on line 102 in internal/cmd/egctl/status.go

Codecov / codecov/patch

internal/cmd/egctl/status.go#L102

Added line #L102 was not covered by tests
return err
}
}
return nil
case "xpolicy":
for _, rt := range supportedXPolicyTypes {
if err = runStatus(ctx, k8sClient, rt, namespace, quiet, verbose, allNamespaces, true, true); err != nil {
if err = runStatus(ctx, cmd.OutOrStdout(), k8sClient, rt, namespace, quiet, verbose, allNamespaces, true, true); err != nil {

Check warning on line 109 in internal/cmd/egctl/status.go

Codecov / codecov/patch

internal/cmd/egctl/status.go#L109

Added line #L109 was not covered by tests
return err
}
}
return nil
default:
return runStatus(ctx, k8sClient, resourceType, namespace, quiet, verbose, allNamespaces, false, false)
return runStatus(ctx, cmd.OutOrStdout(), k8sClient, resourceType, namespace, quiet, verbose, allNamespaces, false, false)

Check warning on line 115 in internal/cmd/egctl/status.go

Codecov / codecov/patch

internal/cmd/egctl/status.go#L115

Added line #L115 was not covered by tests
}
},
}
@@ -138,11 +137,11 @@
}

// runStatus find and write the summary table of status for a specific resource type.
func runStatus(ctx context.Context, cli client.Client, inputResourceType, namespace string, quiet, verbose, allNamespaces, ignoreEmpty, typedName bool) error {
func runStatus(ctx context.Context, logOut io.Writer, cli client.Client, inputResourceType, namespace string, quiet, verbose, allNamespaces, ignoreEmpty, typedName bool) error {

Check warning on line 140 in internal/cmd/egctl/status.go

Codecov / codecov/patch

internal/cmd/egctl/status.go#L140

Added line #L140 was not covered by tests
var (
resourcesList client.ObjectList
resourceKind string
table = newStatusTableWriter(os.Stdout)
table = newStatusTableWriter(logOut)

Check warning on line 144 in internal/cmd/egctl/status.go

Codecov / codecov/patch

internal/cmd/egctl/status.go#L144

Added line #L144 was not covered by tests
)

if allNamespaces {
3 changes: 2 additions & 1 deletion internal/cmd/envoy/shutdown_manager.go
Original file line number Diff line number Diff line change
@@ -21,7 +21,8 @@ import (
"github.com/envoyproxy/gateway/internal/xds/bootstrap"
)

var logger = logging.DefaultLogger(egv1a1.LogLevelInfo).WithName("shutdown-manager")
// TODO: Remove the global logger and localize the scope of the logger.
var logger = logging.DefaultLogger(os.Stdout, egv1a1.LogLevelInfo).WithName("shutdown-manager")

const (
// ShutdownManagerPort is the port Envoy shutdown manager will listen on.
2 changes: 1 addition & 1 deletion internal/cmd/server.go
Original file line number Diff line number Diff line change
@@ -105,7 +105,7 @@ func getConfig(logOut io.Writer) (*config.Server, error) {
// make `cfgPath` an argument to test it without polluting the global var
func getConfigByPath(logOut io.Writer, cfgPath string) (*config.Server, error) {
// Initialize with default config parameters.
cfg, err := config.New()
cfg, err := config.New(logOut)
if err != nil {
return nil, err
}
3 changes: 2 additions & 1 deletion internal/crypto/certgen_test.go
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ import (
"encoding/base64"
"encoding/pem"
"fmt"
"os"
"testing"
"time"

@@ -25,7 +26,7 @@ func TestGenerateCerts(t *testing.T) {
wantEnvoyDNSName string
}

cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

run := func(t *testing.T, name string, tc testcase) {
5 changes: 3 additions & 2 deletions internal/envoygateway/config/config.go
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ package config

import (
"errors"
"io"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/api/v1alpha1/validation"
@@ -41,12 +42,12 @@ type Server struct {
}

// New returns a Server with default parameters.
func New() (*Server, error) {
func New(logOut io.Writer) (*Server, error) {
return &Server{
EnvoyGateway: egv1a1.DefaultEnvoyGateway(),
Namespace: env.Lookup("ENVOY_GATEWAY_NAMESPACE", DefaultNamespace),
DNSDomain: env.Lookup("KUBERNETES_CLUSTER_DOMAIN", DefaultDNSDomain),
Logger: logging.DefaultLogger(egv1a1.LogLevelInfo),
Logger: logging.DefaultLogger(logOut, egv1a1.LogLevelInfo),
Elected: make(chan struct{}),
}, nil
}
5 changes: 3 additions & 2 deletions internal/envoygateway/config/config_test.go
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@
package config

import (
"os"
"testing"

"github.com/stretchr/testify/require"
@@ -21,7 +22,7 @@ var (
)

func TestValidate(t *testing.T) {
cfg, err := New()
cfg, err := New(os.Stdout)
require.NoError(t, err)

testCases := []struct {
@@ -56,7 +57,7 @@ func TestValidate(t *testing.T) {
name: "unspecified envoy gateway",
cfg: &Server{
Namespace: "test-ns",
Logger: logging.DefaultLogger(egv1a1.LogLevelInfo),
Logger: logging.DefaultLogger(os.Stdout, egv1a1.LogLevelInfo),
},
expect: false,
},
2 changes: 1 addition & 1 deletion internal/envoygateway/config/loader/configloader_test.go
Original file line number Diff line number Diff line change
@@ -32,7 +32,7 @@ func TestConfigLoader(t *testing.T) {

cfgPath := tmpDir + "/config.yaml"
require.NoError(t, os.WriteFile(cfgPath, []byte(defaultConfig), 0o600))
s, err := config.New()
s, err := config.New(os.Stdout)
require.NoError(t, err)

ctx, cancel := context.WithCancel(context.TODO())
7 changes: 4 additions & 3 deletions internal/gatewayapi/runner/runner_test.go
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ package runner

import (
"context"
"os"
"reflect"
"testing"
"time"
@@ -30,7 +31,7 @@ func TestRunner(t *testing.T) {
pResources := new(message.ProviderResources)
xdsIR := new(message.XdsIR)
infraIR := new(message.InfraIR)
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)
extMgr, closeFunc, err := registry.NewInMemoryManager(egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
require.NoError(t, err)
@@ -116,7 +117,7 @@ func TestDeleteStatusKeys(t *testing.T) {
pResources := new(message.ProviderResources)
xdsIR := new(message.XdsIR)
infraIR := new(message.InfraIR)
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)
extMgr, closeFunc, err := registry.NewInMemoryManager(egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
require.NoError(t, err)
@@ -217,7 +218,7 @@ func TestDeleteAllStatusKeys(t *testing.T) {
pResources := new(message.ProviderResources)
xdsIR := new(message.XdsIR)
infraIR := new(message.InfraIR)
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)
extMgr, closeFunc, err := registry.NewInMemoryManager(egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
require.NoError(t, err)
3 changes: 2 additions & 1 deletion internal/globalratelimit/runner/runner_test.go
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ package runner
import (
"context"
"fmt"
"os"
"testing"
"time"

@@ -209,7 +210,7 @@ func Test_subscribeAndTranslate(t *testing.T) {
defer cancel()
xdsIR := new(message.XdsIR)
defer xdsIR.Close()
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

r := New(&Config{
5 changes: 3 additions & 2 deletions internal/infrastructure/host/proxy_infra_test.go
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ package host
import (
"bytes"
"context"
"os"
"path"
"testing"

@@ -42,7 +43,7 @@ func newMockInfra(t *testing.T, cfg *config.Server) *Infra {

infra := &Infra{
HomeDir: homeDir,
Logger: logging.DefaultLogger(egv1a1.LogLevelInfo),
Logger: logging.DefaultLogger(os.Stdout, egv1a1.LogLevelInfo),
EnvoyGateway: cfg.EnvoyGateway,
proxyContextMap: make(map[string]*proxyContext),
sdsConfigPath: proxyDir,
@@ -51,7 +52,7 @@ func newMockInfra(t *testing.T, cfg *config.Server) *Infra {
}

func TestInfraCreateProxy(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)
infra := newMockInfra(t, cfg)

Original file line number Diff line number Diff line change
@@ -116,7 +116,7 @@ func newTestInfraWithAnnotationsAndLabels(annotations, labels map[string]string)
}

func TestDeployment(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

cases := []struct {
@@ -645,7 +645,7 @@ func loadDeployment(caseName string) (*appsv1.Deployment, error) {
}

func TestDaemonSet(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

cases := []struct {
@@ -1074,7 +1074,7 @@ func loadDaemonSet(caseName string) (*appsv1.DaemonSet, error) {
}

func TestService(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

svcType := egv1a1.ServiceTypeClusterIP
@@ -1222,7 +1222,7 @@ func loadService(caseName string) (*corev1.Service, error) {
}

func TestConfigMap(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)
cases := []struct {
name string
@@ -1265,7 +1265,7 @@ func loadConfigmap(tc string) (*corev1.ConfigMap, error) {
}

func TestServiceAccount(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)
cases := []struct {
name string
@@ -1308,7 +1308,7 @@ func loadServiceAccount(tc string) (*corev1.ServiceAccount, error) {
}

func TestPDB(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

cases := []struct {
@@ -1414,7 +1414,7 @@ func TestPDB(t *testing.T) {
}

func TestHorizontalPodAutoscaler(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

cases := []struct {
5 changes: 3 additions & 2 deletions internal/infrastructure/kubernetes/proxy_configmap_test.go
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ package kubernetes

import (
"context"
"os"
"testing"

"github.com/stretchr/testify/assert"
@@ -26,7 +27,7 @@ import (
)

func TestCreateOrUpdateProxyConfigMap(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

infra := ir.NewInfra()
@@ -128,7 +129,7 @@ func TestCreateOrUpdateProxyConfigMap(t *testing.T) {
}

func TestDeleteConfigProxyMap(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

infra := ir.NewInfra()
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ package kubernetes

import (
"context"
"os"
"testing"

"github.com/stretchr/testify/require"
@@ -47,7 +48,7 @@ func daemonsetWithSelectorAndLabel(ds *appsv1.DaemonSet, selector *metav1.LabelS
}

func TestCreateOrUpdateProxyDaemonSet(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

infra := ir.NewInfra()
5 changes: 3 additions & 2 deletions internal/infrastructure/kubernetes/proxy_deployment_test.go
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ package kubernetes

import (
"context"
"os"
"testing"

"github.com/stretchr/testify/require"
@@ -52,7 +53,7 @@ func deploymentWithSelectorAndLabel(deploy *appsv1.Deployment, selector *metav1.
}

func TestCreateOrUpdateProxyDeployment(t *testing.T) {
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

infra := ir.NewInfra()
@@ -264,7 +265,7 @@ func TestDeleteProxyDeployment(t *testing.T) {
WithObjects().
WithInterceptorFuncs(interceptorFunc).
Build()
cfg, err := config.New()
cfg, err := config.New(os.Stdout)
require.NoError(t, err)

testCases := []struct {
Loading