Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
6 changes: 4 additions & 2 deletions pkg/mcs/register.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package mcs

import (
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -36,7 +35,10 @@ func addKnownTypes(scheme *runtime.Scheme) error {
func Register(dc *discovery.DiscoveryClient) error {
resources, err := dc.ServerPreferredResources()
if err != nil {
return errors.Wrap(err, "get api groups and resources")
// MCS is optional functionality - if discovery fails for any reason,
// mark it as unavailable and continue without crashing the operator
available = false
return nil
Comment on lines 39 to 43
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we need to log the error and inform users that MCS is not available

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}

outer:
Expand Down
60 changes: 60 additions & 0 deletions pkg/mcs/register_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package mcs

import (
"testing"
)

func TestIsAvailable(t *testing.T) {
// Test IsAvailable function
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are not necessary

available = true
if !IsAvailable() {
t.Error("Expected IsAvailable to return true when available is true")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't we using assert.True(t, IsAvailable()) and instead we use if statements for assertions?

}

available = false
if IsAvailable() {
t.Error("Expected IsAvailable to return false when available is false")
}
}

func TestMCSSchemeGroupVersion(t *testing.T) {
// Test that MCSSchemeGroupVersion is initialized correctly
if MCSSchemeGroupVersion.Group != "" {
t.Errorf("Expected MCSSchemeGroupVersion.Group to be empty initially, got: %s", MCSSchemeGroupVersion.Group)
}
if MCSSchemeGroupVersion.Version != "" {
t.Errorf("Expected MCSSchemeGroupVersion.Version to be empty initially, got: %s", MCSSchemeGroupVersion.Version)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what we are testing here. Since we are not calling a function or something to affect the values, we are essentially asserting that they are empty. Is that what we want?


func TestServiceExport(t *testing.T) {
// Test ServiceExport creation
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed comment

namespace := "test-namespace"
name := "test-service"
labels := map[string]string{
"app": "test",
}

se := ServiceExport(namespace, name, labels)

if se.Name != name {
t.Errorf("Expected ServiceExport name to be %s, got: %s", name, se.Name)
}

if se.Namespace != namespace {
t.Errorf("Expected ServiceExport namespace to be %s, got: %s", namespace, se.Namespace)
}

if se.Labels["app"] != "test" {
t.Errorf("Expected ServiceExport label 'app' to be 'test', got: %s", se.Labels["app"])
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

All these could be assertions


func TestServiceExportList(t *testing.T) {
// Test ServiceExportList creation
seList := ServiceExportList()

if seList.Kind != "ServiceExportList" {
t.Errorf("Expected ServiceExportList Kind to be 'ServiceExportList', got: %s", seList.Kind)
}
}
Comment on lines 31 to 35
Copy link
Contributor

Choose a reason for hiding this comment

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

Same approach with the other tests, we can drop comments and use assertions

Loading