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

Simultaneously support versions v2 and v2beta2 of Autoscaling #1014

Merged
merged 18 commits into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from 17 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
56 changes: 28 additions & 28 deletions apis/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 18 additions & 5 deletions controllers/opentelemetrycollector_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import (

"github.com/go-logr/logr"
appsv1 "k8s.io/api/apps/v1"
autoscalingv1 "k8s.io/api/autoscaling/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -31,6 +32,7 @@ import (

"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
"github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile"
)

Expand Down Expand Up @@ -173,14 +175,25 @@ func (r *OpenTelemetryCollectorReconciler) RunTasks(ctx context.Context, params

// SetupWithManager tells the manager what our controller is interested in.
func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
err := r.config.AutoDetect() // We need to call this so we can get the correct autodetect version
if err != nil {
return err
}
autoscalingVersion := r.config.AutoscalingVersion()
Copy link
Member

Choose a reason for hiding this comment

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

nit: move this closer to the place where it is used - line 192

builder := ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.OpenTelemetryCollector{}).
Owns(&corev1.ConfigMap{}).
Owns(&corev1.ServiceAccount{}).
Owns(&corev1.Service{}).
Owns(&appsv1.Deployment{}).
Owns(&autoscalingv1.HorizontalPodAutoscaler{}).
Owns(&appsv1.DaemonSet{}).
Owns(&appsv1.StatefulSet{}).
Complete(r)
Owns(&appsv1.StatefulSet{})

if autoscalingVersion == autodetect.AutoscalingVersionV2 {
builder = builder.Owns(&autoscalingv2.HorizontalPodAutoscaler{})
} else {
builder = builder.Owns(&autoscalingv2beta2.HorizontalPodAutoscaler{})
}

return builder.Complete(r)
}
29 changes: 27 additions & 2 deletions controllers/opentelemetrycollector_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,21 @@ import (
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/controllers"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
"github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile"
"github.com/open-telemetry/opentelemetry-operator/pkg/platform"
)

var logger = logf.Log.WithName("unit-tests")
var mockAutoDetector = &mockAutoDetect{
HPAVersionFunc: func() (autodetect.AutoscalingVersion, error) {
return autodetect.AutoscalingVersionV2Beta2, nil
},
}

func TestNewObjectsOnReconciliation(t *testing.T) {
// prepare
cfg := config.New(config.WithCollectorImage("default-collector"), config.WithTargetAllocatorImage("default-ta-allocator"))
cfg := config.New(config.WithCollectorImage("default-collector"), config.WithTargetAllocatorImage("default-ta-allocator"), config.WithAutoDetect(mockAutoDetector))
nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
reconciler := controllers.NewReconciler(controllers.Params{
Client: k8sClient,
Expand Down Expand Up @@ -129,7 +136,7 @@ func TestNewObjectsOnReconciliation(t *testing.T) {

func TestNewStatefulSetObjectsOnReconciliation(t *testing.T) {
// prepare
cfg := config.New()
cfg := config.New(config.WithAutoDetect(mockAutoDetector))
nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
reconciler := controllers.NewReconciler(controllers.Params{
Client: k8sClient,
Expand Down Expand Up @@ -341,3 +348,21 @@ func TestRegisterWithManager(t *testing.T) {
// verify
assert.NoError(t, err)
}

var _ autodetect.AutoDetect = (*mockAutoDetect)(nil)

type mockAutoDetect struct {
PlatformFunc func() (platform.Platform, error)
HPAVersionFunc func() (autodetect.AutoscalingVersion, error)
}

func (m *mockAutoDetect) HPAVersion() (autodetect.AutoscalingVersion, error) {
return m.HPAVersionFunc()
}

func (m *mockAutoDetect) Platform() (platform.Platform, error) {
if m.PlatformFunc != nil {
return m.PlatformFunc()
}
return platform.Unknown, nil
}
15 changes: 15 additions & 0 deletions internal/config/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type Config struct {
labelsFilter []string
platform platform.Platform
autoDetectFrequency time.Duration
autoscalingVersion autodetect.AutoscalingVersion
}

// New constructs a new configuration based on the given options.
Expand All @@ -61,6 +62,7 @@ func New(opts ...Option) Config {
logger: logf.Log.WithName("config"),
platform: platform.Unknown,
version: version.Get(),
autoscalingVersion: autodetect.DefaultAutoscalingVersion,
}
for _, opt := range opts {
opt(&o)
Expand All @@ -81,6 +83,7 @@ func New(opts ...Option) Config {
autoInstrumentationPythonImage: o.autoInstrumentationPythonImage,
autoInstrumentationDotNetImage: o.autoInstrumentationDotNetImage,
labelsFilter: o.labelsFilter,
autoscalingVersion: o.autoscalingVersion,
}
}

Expand Down Expand Up @@ -132,6 +135,13 @@ func (c *Config) AutoDetect() error {
}
}

hpaVersion, err := c.autoDetect.HPAVersion()
if err != nil {
return err
}
c.autoscalingVersion = hpaVersion
c.logger.Info("In Autodetect, Set HPA version to [", c.autoscalingVersion, "] from [", hpaVersion, "]")

return nil
}

Expand Down Expand Up @@ -160,6 +170,11 @@ func (c *Config) Platform() platform.Platform {
return c.platform
}

// AutoscalingVersion represents the preferred version of autoscaling.
func (c *Config) AutoscalingVersion() autodetect.AutoscalingVersion {
return c.autoscalingVersion
}

// AutoInstrumentationJavaImage returns OpenTelemetry Java auto-instrumentation container image.
func (c *Config) AutoInstrumentationJavaImage() string {
return c.autoInstrumentationJavaImage
Expand Down
4 changes: 4 additions & 0 deletions internal/config/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ type mockAutoDetect struct {
PlatformFunc func() (platform.Platform, error)
}

func (m *mockAutoDetect) HPAVersion() (autodetect.AutoscalingVersion, error) {
return autodetect.DefaultAutoscalingVersion, nil // TODO add a test?
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove todo?

}

func (m *mockAutoDetect) Platform() (platform.Platform, error) {
if m.PlatformFunc != nil {
return m.PlatformFunc()
Expand Down
1 change: 1 addition & 0 deletions internal/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type options struct {
labelsFilter []string
platform platform.Platform
autoDetectFrequency time.Duration
autoscalingVersion autodetect.AutoscalingVersion
}

func WithAutoDetect(a autodetect.AutoDetect) Option {
Expand Down
62 changes: 62 additions & 0 deletions pkg/autodetect/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
package autodetect

import (
"errors"
"sort"

"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"

Expand All @@ -27,12 +30,23 @@ var _ AutoDetect = (*autoDetect)(nil)
// AutoDetect provides an assortment of routines that auto-detect traits based on the runtime.
type AutoDetect interface {
Platform() (platform.Platform, error)
HPAVersion() (AutoscalingVersion, error)
}

type autoDetect struct {
dcl discovery.DiscoveryInterface
}

type AutoscalingVersion int

const (
AutoscalingVersionV2 AutoscalingVersion = iota
AutoscalingVersionV2Beta2
AutoscalingVersionUnknown
)

const DefaultAutoscalingVersion = AutoscalingVersionV2

// New creates a new auto-detection worker, using the given client when talking to the current cluster.
func New(restConfig *rest.Config) (AutoDetect, error) {
dcl, err := discovery.NewDiscoveryClientForConfig(restConfig)
Expand Down Expand Up @@ -64,3 +78,51 @@ func (a *autoDetect) Platform() (platform.Platform, error) {

return platform.Kubernetes, nil
}

func (a *autoDetect) HPAVersion() (AutoscalingVersion, error) {
apiList, err := a.dcl.ServerGroups()
if err != nil {
return AutoscalingVersionUnknown, err
}

for _, apiGroup := range apiList.Groups {
if apiGroup.Name == "autoscaling" {
// Sort this so we can make sure to get v2 before v2beta2
versions := apiGroup.Versions
sort.Slice(versions, func(i, j int) bool {
return versions[i].Version < versions[j].Version
})

for _, version := range versions {
if version.Version == "v2" || version.Version == "v2beta2" {
return toAutoscalingVersion(version.Version), nil
}
}
return AutoscalingVersionUnknown, errors.New("Failed to find appropriate version of apiGroup autoscaling, only v2 and v2beta2 are supported")
}
}

return AutoscalingVersionUnknown, errors.New("Failed to find apiGroup autoscaling")
}

func (v AutoscalingVersion) String() string {
switch v {
case AutoscalingVersionV2:
return "v2"
case AutoscalingVersionV2Beta2:
return "v2beta2"
case AutoscalingVersionUnknown:
return "unknown"
}
return "unknown"
}

func toAutoscalingVersion(version string) AutoscalingVersion {
switch version {
case "v2":
return AutoscalingVersionV2Beta2
case "v2beta2":
return AutoscalingVersionV2Beta2
}
return AutoscalingVersionUnknown
}
6 changes: 6 additions & 0 deletions pkg/autodetect/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,9 @@ func TestUnknownPlatformOnError(t *testing.T) {
assert.Error(t, err)
assert.Equal(t, platform.Unknown, plt)
}

func TestAutoscalingVersionToString(t *testing.T) {
assert.Equal(t, "v2", autodetect.AutoscalingVersionV2.String())
assert.Equal(t, "v2beta2", autodetect.AutoscalingVersionV2Beta2.String())
assert.Equal(t, "unknown", autodetect.AutoscalingVersionUnknown.String())
}
Loading