Skip to content

Commit

Permalink
post rebase fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Thibault Mange <[email protected]>
  • Loading branch information
@thibaultmg
thibaultmg committed Jun 18, 2024
1 parent 9f11218 commit af2e428
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 90 deletions.
9 changes: 5 additions & 4 deletions operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/microshift"
"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/openshift"
"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/rendering"
"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/status"
"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/util"
oav1beta1 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta1"
oav1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2"
Expand Down Expand Up @@ -198,8 +199,8 @@ func (r *ObservabilityAddonReconciler) Reconcile(ctx context.Context, req ctrl.R
log.Error(err, "OCP prometheus service does not exist")
// ACM 8509: Special case for hub/local cluster metrics collection
// We do not report status for hub endpoint operator
if !isHubMetricsCollector {
if err := util.ReportStatus(ctx, r.Client, util.NotSupported, obsAddon.Name, obsAddon.Namespace); err != nil {
if !r.IsHubMetricsCollector {
if err := status.ReportStatus(ctx, r.Client, status.NotSupported, obsAddon.Name, obsAddon.Namespace); err != nil {
log.Error(err, "Failed to report status")
}
}
Expand Down Expand Up @@ -263,8 +264,8 @@ func (r *ObservabilityAddonReconciler) Reconcile(ctx context.Context, req ctrl.R
deployer := deploying.NewDeployer(r.Client)

// Ordering resources to ensure they are applied in the correct order
slices.SortFunc(toDeploy, func(a, b *unstructured.Unstructured) bool {
return (resourcePriority(a) - resourcePriority(b)) < 0
slices.SortFunc(toDeploy, func(a, b *unstructured.Unstructured) int {
return resourcePriority(a) - resourcePriority(b)
})

for _, res := range toDeploy {
Expand Down
33 changes: 4 additions & 29 deletions ...trics/controllers/observabilityendpoint/observabilityaddon_controller_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ func TestIntegrationReconcileHypershift(t *testing.T) {
t.Fatalf("Failed to create resources: %v", err)
}

mgr, err := ctrl.NewManager(testEnv.Config, ctrl.Options{
mgr, err := ctrl.NewManager(testEnvHub.Config, ctrl.Options{
Scheme: k8sClient.Scheme(),
Metrics: metricsserver.Options{BindAddress: "0"},
Metrics: metricsserver.Options{BindAddress: "0"}, // Avoids port conflict with the default port 8080
})
assert.NoError(t, err)

Expand Down Expand Up @@ -175,8 +175,8 @@ func TestIntegrationReconcileMicroshift(t *testing.T) {
}

mgr, err := ctrl.NewManager(testEnvSpoke.Config, ctrl.Options{
Scheme: k8sClientSpoke.Scheme(),
MetricsBindAddress: "0", // Avoids port conflict with the default port 8080
Scheme: k8sClientSpoke.Scheme(),
Metrics: metricsserver.Options{BindAddress: "0"}, // Avoids port conflict with the default port 8080
})
assert.NoError(t, err)

Expand Down Expand Up @@ -285,31 +285,6 @@ func createBaseScheme() *runtime.Scheme {
return scheme
}

func setupManager(t *testing.T, k8sClient client.Client) {
mgr, err := ctrl.NewManager(testEnvSpoke.Config, ctrl.Options{
Scheme: k8sClient.Scheme(),
MetricsBindAddress: "0", // Avoids port conflict with the default port 8080
})
assert.NoError(t, err)

hubClientWithReload, err := util.NewReloadableHubClientWithReloadFunc(func() (client.Client, error) {
return k8sClient, nil
})
assert.NoError(t, err)
reconciler := ObservabilityAddonReconciler{
Client: k8sClient,
HubClient: hubClientWithReload,
}

err = reconciler.SetupWithManager(mgr)
assert.NoError(t, err)

go func() {
err = mgr.Start(ctrl.SetupSignalHandler())
assert.NoError(t, err)
}()
}

func setupCommonHubResources(t *testing.T, k8sClient client.Client, ns string) {
// Create resources required for the observability addon controller
resourcesDeps := []client.Object{
Expand Down
49 changes: 12 additions & 37 deletions ...s/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,17 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
clusterv1 "open-cluster-management.io/api/cluster/v1"
kubescheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"

"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/openshift"
"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/util"
oashared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared"
oav1beta1 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta1"
mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2"
operatorconfig "github.com/stolostron/multicluster-observability-operator/operators/pkg/config"
addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
)

const (
Expand Down Expand Up @@ -143,12 +140,6 @@ func newImagesCM(ns string) *corev1.ConfigMap {

func init() {
os.Setenv("UNIT_TEST", "true")
s := scheme.Scheme
addonv1alpha1.AddToScheme(s)
oav1beta1.AddToScheme(s)
ocinfrav1.AddToScheme(s)
hyperv1.AddToScheme(s)
promv1.AddToScheme(s)

namespace = testNamespace
hubNamespace = testHubNamspace
Expand Down Expand Up @@ -192,32 +183,16 @@ alertmanager-router-ca: |
},
}

scheme := scheme.Scheme
addonv1alpha1.AddToScheme(scheme)
mcov1beta2.AddToScheme(scheme)
oav1beta1.AddToScheme(scheme)
corev1.AddToScheme(scheme)
clusterv1.AddToScheme(scheme)
ocinfrav1.AddToScheme(scheme)
s := runtime.NewScheme()
kubescheme.AddToScheme(s)
addonv1alpha1.AddToScheme(s)
oav1beta1.AddToScheme(s)
ocinfrav1.AddToScheme(s)
hyperv1.AddToScheme(s)
promv1.AddToScheme(s)

hubClient := fake.NewClientBuilder().
WithScheme(scheme).
WithRuntimeObjects(hubObjs...).
WithStatusSubresource(
&addonv1alpha1.ManagedClusterAddOn{},
&mcov1beta2.MultiClusterObservability{},
&oav1beta1.ObservabilityAddon{},
).
Build()
c := fake.NewClientBuilder().
WithScheme(scheme).
WithRuntimeObjects(objs...).
WithStatusSubresource(
&addonv1alpha1.ManagedClusterAddOn{},
&mcov1beta2.MultiClusterObservability{},
&oav1beta1.ObservabilityAddon{},
).
Build()
hubClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(hubObjs...).Build()
c := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(objs...).Build()

hubClientWithReload, err := util.NewReloadableHubClientWithReloadFunc(func() (client.Client, error) {
return hubClient, nil
Expand All @@ -228,7 +203,7 @@ alertmanager-router-ca: |
r := &ObservabilityAddonReconciler{
Client: c,
HubClient: hubClientWithReload,
Scheme: scheme.Scheme,
Scheme: s,
IsHubMetricsCollector: false,
}

Expand Down
2 changes: 1 addition & 1 deletion operators/endpointmetrics/pkg/rendering/renderer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func Render(
return resources, nil
}

func getDisabledMetrics(c runtimeclient.Client) (string, error) {
func getDisabledMetrics(ctx context.Context, c runtimeclient.Client) (string, error) {
cm := &corev1.ConfigMap{}
err := c.Get(ctx, types.NamespacedName{Name: operatorconfig.AllowlistConfigMapName,
Namespace: namespace}, cm)
Expand Down
3 changes: 2 additions & 1 deletion operators/endpointmetrics/pkg/rendering/renderer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
operatorconfig "github.com/stolostron/multicluster-observability-operator/operators/pkg/config"
rendererutil "github.com/stolostron/multicluster-observability-operator/operators/pkg/rendering"
templatesutil "github.com/stolostron/multicluster-observability-operator/operators/pkg/rendering/templates"
"github.com/stretchr/testify/assert"
)

func getAllowlistCM() *corev1.ConfigMap {
Expand Down Expand Up @@ -57,5 +58,5 @@ func TestRender(t *testing.T) {
t.Fatalf("failed to render endpoint templates: %v", err)
}

printObjs(t, objs)
assert.Greater(t, len(objs), 2)
}
37 changes: 19 additions & 18 deletions operators/endpointmetrics/pkg/status/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"
"time"

"github.com/stolostron/multicluster-observability-operator/operators/endpointmetrics/pkg/status"
oav1beta1 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta1"
mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -44,15 +45,15 @@ func TestReportStatus(t *testing.T) {

testCases := map[string]struct {
currentConditions []oav1beta1.StatusCondition
newCondition util.ConditionReason
newCondition status.ConditionReason
expects func(*testing.T, []oav1beta1.StatusCondition)
}{
"new status should be appended": {
currentConditions: []oav1beta1.StatusCondition{},
newCondition: util.Deployed,
newCondition: status.Deployed,
expects: func(t *testing.T, conditions []oav1beta1.StatusCondition) {
assert.Len(t, conditions, 1)
assert.EqualValues(t, util.Deployed, conditions[0].Reason)
assert.EqualValues(t, status.Deployed, conditions[0].Reason)
assert.Equal(t, metav1.ConditionTrue, conditions[0].Status)
assert.Equal(t, "Progressing", conditions[0].Type)
assert.InEpsilon(t, time.Now().Unix(), conditions[0].LastTransitionTime.Unix(), 1)
Expand All @@ -62,7 +63,7 @@ func TestReportStatus(t *testing.T) {
currentConditions: []oav1beta1.StatusCondition{
{
Type: "Progressing",
Reason: string(util.Deployed),
Reason: string(status.Deployed),
Message: "Metrics collector deployed",
Status: metav1.ConditionTrue,
LastTransitionTime: metav1.Time{
Expand All @@ -71,28 +72,28 @@ func TestReportStatus(t *testing.T) {
},
{
Type: "Disabled",
Reason: string(util.Disabled),
Reason: string(status.Disabled),
Message: "enableMetrics is set to False",
Status: metav1.ConditionTrue,
LastTransitionTime: metav1.Time{
Time: time.Now().Add(-2 * time.Minute),
},
},
},
newCondition: util.Disabled,
newCondition: status.Disabled,
expects: func(t *testing.T, conditions []oav1beta1.StatusCondition) {
assert.Len(t, conditions, 2)
found := false
for _, c := range conditions {
if c.Reason == string(util.Disabled) {
if c.Reason == string(status.Disabled) {
found = true
assert.EqualValues(t, util.Disabled, c.Reason)
assert.EqualValues(t, status.Disabled, c.Reason)
assert.Equal(t, metav1.ConditionTrue, c.Status)
assert.Equal(t, "Disabled", c.Type)
assert.InEpsilon(t, time.Now().Unix(), c.LastTransitionTime.Unix(), 1)
} else {
// other condition should not be changed
assert.EqualValues(t, util.Deployed, c.Reason)
assert.EqualValues(t, status.Deployed, c.Reason)
assert.InEpsilon(t, time.Now().Add(-time.Minute).Unix(), c.LastTransitionTime.Unix(), 1)
}
}
Expand All @@ -103,7 +104,7 @@ func TestReportStatus(t *testing.T) {
currentConditions: []oav1beta1.StatusCondition{
{
Type: "Progressing",
Reason: string(util.Deployed),
Reason: string(status.Deployed),
Message: "Metrics collector deployed",
Status: metav1.ConditionTrue,
LastTransitionTime: metav1.Time{
Expand All @@ -117,10 +118,10 @@ func TestReportStatus(t *testing.T) {
},
},
},
newCondition: util.Deployed,
newCondition: status.Deployed,
expects: func(t *testing.T, conditions []oav1beta1.StatusCondition) {
assert.Len(t, conditions, 2)
assert.EqualValues(t, util.Deployed, conditions[0].Reason)
assert.EqualValues(t, status.Deployed, conditions[0].Reason)
assert.InEpsilon(t, time.Now().Add(-time.Minute).Unix(), conditions[0].LastTransitionTime.Unix(), 1)
},
},
Expand All @@ -129,10 +130,10 @@ func TestReportStatus(t *testing.T) {
{Type: "1"}, {Type: "2"}, {Type: "3"}, {Type: "4"}, {Type: "5"},
{Type: "6"}, {Type: "7"}, {Type: "8"}, {Type: "9"}, {Type: "10"},
},
newCondition: util.Deployed,
newCondition: status.Deployed,
expects: func(t *testing.T, conditions []oav1beta1.StatusCondition) {
assert.Len(t, conditions, util.MaxStatusConditionsCount)
assert.EqualValues(t, util.Deployed, conditions[len(conditions)-1].Reason)
assert.Len(t, conditions, status.MaxStatusConditionsCount)
assert.EqualValues(t, status.Deployed, conditions[len(conditions)-1].Reason)
},
},
"duplicated conditions should be removed": {
Expand All @@ -142,7 +143,7 @@ func TestReportStatus(t *testing.T) {
{Type: "Progressing", LastTransitionTime: metav1.Time{Time: time.Now().Add(-time.Minute)}},
{Type: "Degraded", LastTransitionTime: metav1.Time{Time: time.Now().Add(-time.Minute)}},
},
newCondition: util.Deployed,
newCondition: status.Deployed,
expects: func(t *testing.T, conditions []oav1beta1.StatusCondition) {
assert.Len(t, conditions, 2)
for _, c := range conditions {
Expand Down Expand Up @@ -172,7 +173,7 @@ func TestReportStatus(t *testing.T) {
}

// test
if err := util.ReportStatus(context.Background(), client, tc.newCondition, baseAddon.Name, baseAddon.Namespace); err != nil {
if err := status.ReportStatus(context.Background(), client, tc.newCondition, baseAddon.Name, baseAddon.Namespace); err != nil {
t.Fatalf("Error reporting status: %v", err)
}
newAddon := &oav1beta1.ObservabilityAddon{}
Expand Down Expand Up @@ -200,7 +201,7 @@ func TestReportStatus_Conflict(t *testing.T) {
conflictErr := errors.NewConflict(schema.GroupResource{Group: oav1beta1.GroupVersion.Group, Resource: "resource"}, name, fmt.Errorf("conflict"))

c := newClientWithUpdateError(fakeClient, conflictErr)
if err := util.ReportStatus(context.Background(), c, util.Deployed, name, testNamespace); err == nil {
if err := status.ReportStatus(context.Background(), c, status.Deployed, name, testNamespace); err == nil {
t.Fatalf("Conflict error should be retried and return an error if it fails")
}
if c.UpdateCallsCount() <= 1 {
Expand Down

0 comments on commit af2e428

Please sign in to comment.