Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Fix bug in which APIServerTracing did not work with some egress selectors #112979

Merged
merged 1 commit into from Oct 18, 2022
Merged

Fix bug in which APIServerTracing did not work with some egress selectors #112979

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions staging/src/k8s.io/apiserver/pkg/server/options/tracing.go
View file
Open in desktop
Expand Up @@ -101,11 +101,12 @@ func (o *TracingOptions) ApplyTo(es *egressselector.EgressSelector, c *server.Co
if err != nil {
return err
}

otelDialer := func(ctx context.Context, addr string) (net.Conn, error) {
return egressDialer(ctx, "tcp", addr)
if egressDialer != nil {
Copy link

Choose a reason for hiding this comment

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

what is going to happen if egressDialer is nil?

Copy link
Contributor Author

@dashpole dashpole Oct 18, 2022 •
edited

Choose a reason for hiding this comment

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

It won't use a custom dialer in that case (it will just use the default dial function).

Edit: it is equivalent to not having an egress selector at all.

Copy link
Member

Choose a reason for hiding this comment

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

huh... probably worth noting loudly in the Lookup documentation that this can return nil, nil (and maybe sweeping for other callers that assume a nil error means a non-nil dialer)

otelDialer := func(ctx context.Context, addr string) (net.Conn, error) {
return egressDialer(ctx, "tcp", addr)
}
opts = append(opts, otlptracegrpc.WithDialOption(grpc.WithContextDialer(otelDialer)))
}
opts = append(opts, otlptracegrpc.WithDialOption(grpc.WithContextDialer(otelDialer)))
}

resourceOpts := []resource.Option{
Expand Down
75 changes: 64 additions & 11 deletions test/integration/apiserver/tracing/tracing_test.go
View file
Open in desktop
Expand Up @@ -41,24 +41,56 @@ import (
)

func TestAPIServerTracing(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.APIServerTracing, true)()

// Listen for traces from the API Server before starting it, so the
// API Server will successfully connect right away during the test.
listener, err := net.Listen("tcp", "localhost:")
if err != nil {
t.Fatal(err)
}
// Write the configuration for tracing to a file
tracingConfigFile, err := os.CreateTemp("", "tracing-config.yaml")
if err != nil {
t.Fatal(err)
}
defer os.Remove(tracingConfigFile.Name())

traceFound := make(chan struct{})
defer close(traceFound)
srv := grpc.NewServer()
traceservice.RegisterTraceServiceServer(srv, &traceServer{
traceFound: traceFound,
filterFunc: containsNodeListSpan})
if err := os.WriteFile(tracingConfigFile.Name(), []byte(fmt.Sprintf(`
apiVersion: apiserver.config.k8s.io/v1alpha1
kind: TracingConfiguration
endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil {
t.Fatal(err)
}
testAPIServerTracing(t,
listener,
[]string{"--tracing-config-file=" + tracingConfigFile.Name()},
)
}

go srv.Serve(listener)
defer srv.Stop()
func TestAPIServerTracingWithEgressSelector(t *testing.T) {
// Listen for traces from the API Server before starting it, so the
// API Server will successfully connect right away during the test.
listener, err := net.Listen("tcp", "localhost:")
if err != nil {
t.Fatal(err)
}
// Use an egress selector which doesn't have a controlplane config to ensure
// tracing works in that context. Write the egress selector configuration to a file.
egressSelectorConfigFile, err := os.CreateTemp("", "egress_selector_configuration.yaml")
if err != nil {
t.Fatal(err)
}
defer os.Remove(egressSelectorConfigFile.Name())

if err := os.WriteFile(egressSelectorConfigFile.Name(), []byte(`
apiVersion: apiserver.config.k8s.io/v1beta1
kind: EgressSelectorConfiguration
egressSelections:
- name: cluster
connection:
proxyProtocol: Direct
transport:`), os.FileMode(0755)); err != nil {
t.Fatal(err)
}

// Write the configuration for tracing to a file
tracingConfigFile, err := os.CreateTemp("", "tracing-config.yaml")
Expand All @@ -73,11 +105,32 @@ kind: TracingConfiguration
endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil {
t.Fatal(err)
}
testAPIServerTracing(t,
listener,
[]string{
"--tracing-config-file=" + tracingConfigFile.Name(),
"--egress-selector-config-file=" + egressSelectorConfigFile.Name(),
},
)
}

func testAPIServerTracing(t *testing.T, listener net.Listener, apiserverArgs []string) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.APIServerTracing, true)()

traceFound := make(chan struct{})
defer close(traceFound)
srv := grpc.NewServer()
traceservice.RegisterTraceServiceServer(srv, &traceServer{
traceFound: traceFound,
filterFunc: containsNodeListSpan})

go srv.Serve(listener)
defer srv.Stop()

// Start the API Server with our tracing configuration
testServer := kubeapiservertesting.StartTestServerOrDie(t,
kubeapiservertesting.NewDefaultTestServerOptions(),
[]string{"--tracing-config-file=" + tracingConfigFile.Name()},
apiserverArgs,
framework.SharedEtcd(),
)
defer testServer.TearDownFn()
Expand Down