e2e-tests (3/4): Add gNMI integration tests with envtest

[nikatza]

This PR introduces envtest-based gNMI integration tests for the OpenConfig provider, enabling faster validation of the Kubernetes resources to gNMI sequence without requiring a full cluster deployment.

The test architecture uses a per-process gNMI server design that enables parallel test execution with Ginkgo's --procs flag. Each process creates its own suite-level server in BeforeSuite and shares it across tests running serially within that process.

[github-actions]

Merging this branch will not change overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/ironcore-dev/network-operator/test/gnmi	0.00% (ø)
github.com/ironcore-dev/network-operator/test/gnmi/server	0.00% (ø)

---

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/ironcore-dev/network-operator/test/gnmi/server/server.go	0.00% (ø)	0	0	0

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

• github.com/ironcore-dev/network-operator/test/gnmi/gnmi_suite_test.go
• github.com/ironcore-dev/network-operator/test/gnmi/gnmi_test.go
• github.com/ironcore-dev/network-operator/test/gnmi/json_test.go

[IvoGoman]

Proposal for a bit more aggressive filtering. This should only trigger e2e on code or dependency changes.

Suggested change

	paths-ignore:
	- 'docs/**'
	- '**/*.md'
	paths:
	- '**.go'
	- 'hack/**'
	- 'config/**'
	- 'go.mod'
	- 'go.sum'
	- 'Makefile'

[IvoGoman]

This can be rewritten using only gomega matchers:

Suggested change

	if envProvider == "" {
	It("requires PROVIDER to be set", func() {
	Fail(ProviderEnvVar + " not set. Please set " + ProviderEnvVar + " to one of: " + strings.Join(supportedProviders, ", "))
	})
	return
	}
	Expect(envProvider).NotTo(BeEmpty(), "PROVIDER environment variable must be set to one of: %s", strings.Join(supportedProviders, ", "))

Same for some other if checks in this file

[IvoGoman]

Have you tried the gomega build-in matcher for JSON? https://onsi.github.io/gomega/#matchjsonjson-any

[felix-kaestner]

Suggested change

	name: Test gNMI integration
	name: Test gNMI integration

why does this file have a two space indentation by default?

[felix-kaestner]

Is this install step required if the 'make test-gnmi' already takes care of installing ginkgo if it's not present?

[felix-kaestner]

Suggested change

	name: Run gNMI Integration Tests (OpenConfig)
	name: gNMI Integration (${{ matrix.provider }})
	strategy:
	fail-fast: false
	matrix:
	provider:
	- openconfig

Should we already integrate a matrix build, such that it is easier to integrate more providers later?

[felix-kaestner]

Suggested change

	run: make test-gnmi TESTGNMI_PROVIDER=openconfig GINKGO_PROCS=4
	run: make test-gnmi TESTGNMI_PROVIDER=openconfig GINKGO_PROCS=$(nproc)

why not let the platform use all available?

[felix-kaestner]

Suggested change

	if s.grpcServer != nil {
	s.grpcServer.GracefulStop()
	}
	if s.grpcServer != nil {
	const timeout = 5 * time.Second
	t := time.AfterFunc(timeout, func() {
	log.Printf("Graceful shutdown timed out, forcing stop")
	s.grpcServer.Stop()
	})
	s.grpcServer.GracefulStop()
	t.Stop()
	}

What about this? This completely removes the need to create a done chan and spawing a gorountine in a goroutine. So the go func in the NewTestServer can just become:

	go func() {
		<-ctx.Done()
		server.Close()
	}()

[felix-kaestner]

I guess a

c.Delete(cleanupCtx, device, client.PropagationPolicy(metav1.DeletePropagationForeground))

should already work for deleting in the right order.

If that is not the case, then we clearly have some internal errors elsewhere.

[felix-kaestner]

I think this is a race, as ggnmiServerCancel()triggers a context cancelation, which means the the goroutine inside ofNewTestServerwill continue to callGracefulStop(), and Close()later also callsGracefulStop(). I believe this is also the reason why sync.Once` was introduced as a quickfix, but I feel like that shows that the implementation is a bit muddy here.

[felix-kaestner]

Why two separate contexts? Isn't this just one from t.Context()?

[felix-kaestner]

Suggested change

	func (s *State) Set(path *gpb.Path, raw []byte) error {
	func (s *State) Set(path *gpb.Path, raw []byte) (err error) {

nit: avoid having multiple var err error later.

[felix-kaestner]

Suggested change

	elems := path.GetElem()
	if len(elems) == 0 {
	s.Buf = buf
	return nil
	}
	for k, v := range elems[len(elems)-1].GetKey() {
	buf, err = sjson.SetBytes(buf, sb.String()+"."+k, v)
	if err != nil {
	return fmt.Errorf("sjson.SetBytes final key %s: %w", k, err)
	}
	}
	if elems := path.GetElem(); len(elems) > 0 {
	for k, v := range elems[len(elems)-1].GetKey() {
	buf, err = sjson.SetBytes(buf, sb.String()+"."+k, v)
	if err != nil {
	return fmt.Errorf("sjson.SetBytes final key %s: %w", k, err)
	}
	}
	}