From cfa91397af0300303f1c6cc10c977adb763d56e5 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 16 Nov 2021 11:57:31 +0100 Subject: [PATCH 1/3] Add upgrade procedure for Instrumentation Signed-off-by: Pavol Loffay --- pkg/instrumentation/upgrade/upgrade.go | 61 ++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 pkg/instrumentation/upgrade/upgrade.go diff --git a/pkg/instrumentation/upgrade/upgrade.go b/pkg/instrumentation/upgrade/upgrade.go new file mode 100644 index 0000000000..1d0e820ff2 --- /dev/null +++ b/pkg/instrumentation/upgrade/upgrade.go @@ -0,0 +1,61 @@ +package upgrade + +import ( + "context" + "fmt" + "github.com/Masterminds/semver/v3" + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "strings" + + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/open-telemetry/opentelemetry-operator/internal/version" +) + +func ManagedInstances(ctx context.Context, logger logr.Logger, ver version.Version, cl client.Client) error { + logger.Info("looking for managed Instrumentation instances to upgrade") + + opts := []client.ListOption{ + client.MatchingLabels(map[string]string{ + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }), + } + list := &v1alpha1.InstrumentationList{} + if err := cl.List(ctx, list, opts...); err != nil { + return fmt.Errorf("failed to list: %w", err) + } + + for i := range list.Items { + instToUpgrade := list.Items[i] + err := upgrade(ctx, ver, instToUpgrade) + if err != nil { + // nothing to do + continue + } + } +} + +func upgrade(ctx context.Context, version version.Version, inst v1alpha1.Instrumentation) (v1alpha1.Instrumentation, error) { + autoJavaSemver, err := semver.NewVersion(version.JavaAutoInstrumentation) + if err != nil { + return v1alpha1.Instrumentation{}, err + } + + autoInstJava := inst.Annotations[v1alpha1.AnnotationDefaultAutoInstrumentationJava] + if autoInstJava != "" { + index := strings.Index(autoInstJava, ":") + if index == -1 { + return inst, fmt.Errorf("cannot upgrade") + } + instanceV, err := semver.NewVersion(autoInstJava[index:]) + if err != nil { + return inst, err + } + + if instanceV.LessThan(autoJavaSemver) { + instanceV.String() + } + } +} + From aad50568955f6d7a096e1fe33b70b8049a203f70 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 16 Nov 2021 16:24:43 +0100 Subject: [PATCH 2/3] Fix Signed-off-by: Pavol Loffay --- apis/v1alpha1/instrumentation_webhook.go | 2 +- ...emetry-operator.clusterserviceversion.yaml | 4 + config/rbac/role.yaml | 4 + main.go | 17 ++++- pkg/instrumentation/upgrade/upgrade.go | 75 +++++++++++------- .../upgrade/upgrade_suite_test.go | 67 ++++++++++++++++ pkg/instrumentation/upgrade/upgrade_test.go | 76 +++++++++++++++++++ 7 files changed, 213 insertions(+), 32 deletions(-) create mode 100644 pkg/instrumentation/upgrade/upgrade_suite_test.go create mode 100644 pkg/instrumentation/upgrade/upgrade_test.go diff --git a/apis/v1alpha1/instrumentation_webhook.go b/apis/v1alpha1/instrumentation_webhook.go index 0db375d499..626d9de166 100644 --- a/apis/v1alpha1/instrumentation_webhook.go +++ b/apis/v1alpha1/instrumentation_webhook.go @@ -21,7 +21,7 @@ import ( ) const ( - AnnotationDefaultAutoInstrumentationJava = "opentelemetry.io/default-auto-instrumentation-java-image" + AnnotationDefaultAutoInstrumentationJava = "instrumentation.opentelemetry.io/default-auto-instrumentation-java-image" ) // log is for logging in this package. diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index 8b47a71b25..cbd2f63eaa 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -171,8 +171,12 @@ spec: resources: - instrumentations verbs: + - create + - delete - get - list + - patch + - update - watch - apiGroups: - opentelemetry.io diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 328ef1e583..0c131d382f 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -106,8 +106,12 @@ rules: resources: - instrumentations verbs: + - create + - delete - get - list + - patch + - update - watch - apiGroups: - opentelemetry.io diff --git a/main.go b/main.go index 5d9d5b46b3..f70438c099 100644 --- a/main.go +++ b/main.go @@ -40,8 +40,9 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/version" "github.com/open-telemetry/opentelemetry-operator/internal/webhookhandler" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade" + collectorupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade" "github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation" + instrumentationupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation/upgrade" "github.com/open-telemetry/opentelemetry-operator/pkg/sidecar" // +kubebuilder:scaffold:imports ) @@ -152,7 +153,19 @@ func main() { // adds the upgrade mechanism to be executed once the manager is ready err = mgr.Add(manager.RunnableFunc(func(c context.Context) error { - return upgrade.ManagedInstances(c, ctrl.Log.WithName("upgrade"), v, mgr.GetClient()) + return collectorupgrade.ManagedInstances(c, ctrl.Log.WithName("collector-upgrade"), v, mgr.GetClient()) + })) + if err != nil { + setupLog.Error(err, "failed to upgrade managed instances") + } + // adds the upgrade mechanism to be executed once the manager is ready + err = mgr.Add(manager.RunnableFunc(func(c context.Context) error { + u := &instrumentationupgrade.InstrumentationUpgrade{ + Logger: ctrl.Log.WithName("instrumentation-upgrade"), + DefaultAutoInstrJava: autoInstrumentationJava, + Client: mgr.GetClient(), + } + return u.ManagedInstances(c) })) if err != nil { setupLog.Error(err, "failed to upgrade managed instances") diff --git a/pkg/instrumentation/upgrade/upgrade.go b/pkg/instrumentation/upgrade/upgrade.go index 1d0e820ff2..402c40d4d6 100644 --- a/pkg/instrumentation/upgrade/upgrade.go +++ b/pkg/instrumentation/upgrade/upgrade.go @@ -1,20 +1,41 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package upgrade import ( "context" "fmt" - "github.com/Masterminds/semver/v3" - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "strings" + "reflect" "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/open-telemetry/opentelemetry-operator/internal/version" + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" ) -func ManagedInstances(ctx context.Context, logger logr.Logger, ver version.Version, cl client.Client) error { - logger.Info("looking for managed Instrumentation instances to upgrade") +type InstrumentationUpgrade struct { + Logger logr.Logger + DefaultAutoInstrJava string + Client client.Client +} + +//+kubebuilder:rbac:groups=opentelemetry.io,resources=instrumentations,verbs=get;list;watch;create;update;patch;delete + +// ManagedInstances upgrades managed instances by the opentelemetry-operator. +func (u *InstrumentationUpgrade) ManagedInstances(ctx context.Context) error { + u.Logger.Info("looking for managed Instrumentation instances to upgrade") opts := []client.ListOption{ client.MatchingLabels(map[string]string{ @@ -22,40 +43,36 @@ func ManagedInstances(ctx context.Context, logger logr.Logger, ver version.Versi }), } list := &v1alpha1.InstrumentationList{} - if err := cl.List(ctx, list, opts...); err != nil { + if err := u.Client.List(ctx, list, opts...); err != nil { return fmt.Errorf("failed to list: %w", err) } for i := range list.Items { - instToUpgrade := list.Items[i] - err := upgrade(ctx, ver, instToUpgrade) - if err != nil { - // nothing to do - continue + toUpgrade := list.Items[i] + upgraded := u.upgrade(ctx, toUpgrade) + if !reflect.DeepEqual(upgraded, toUpgrade) { + // use update instead of patch because the patch does not upgrade annotations + if err := u.Client.Update(ctx, &upgraded); err != nil { + u.Logger.Error(err, "failed to apply changes to instance", "name", upgraded.Name, "namespace", upgraded.Namespace) + continue + } } } -} -func upgrade(ctx context.Context, version version.Version, inst v1alpha1.Instrumentation) (v1alpha1.Instrumentation, error) { - autoJavaSemver, err := semver.NewVersion(version.JavaAutoInstrumentation) - if err != nil { - return v1alpha1.Instrumentation{}, err + if len(list.Items) == 0 { + u.Logger.Info("no instances to upgrade") } + return nil +} +func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instrumentation) v1alpha1.Instrumentation { autoInstJava := inst.Annotations[v1alpha1.AnnotationDefaultAutoInstrumentationJava] if autoInstJava != "" { - index := strings.Index(autoInstJava, ":") - if index == -1 { - return inst, fmt.Errorf("cannot upgrade") - } - instanceV, err := semver.NewVersion(autoInstJava[index:]) - if err != nil { - return inst, err - } - - if instanceV.LessThan(autoJavaSemver) { - instanceV.String() + // upgrade the image only if the image matches the annotation + if inst.Spec.Java.Image == autoInstJava { + inst.Spec.Java.Image = u.DefaultAutoInstrJava + inst.Annotations[v1alpha1.AnnotationDefaultAutoInstrumentationJava] = u.DefaultAutoInstrJava } } + return inst } - diff --git a/pkg/instrumentation/upgrade/upgrade_suite_test.go b/pkg/instrumentation/upgrade/upgrade_suite_test.go new file mode 100644 index 0000000000..fb3891f9ec --- /dev/null +++ b/pkg/instrumentation/upgrade/upgrade_suite_test.go @@ -0,0 +1,67 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package upgrade + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" +) + +var k8sClient client.Client +var testEnv *envtest.Environment +var testScheme = scheme.Scheme + +func TestMain(m *testing.M) { + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("..", "..", "..", "config", "crd", "bases"), + }, + } + + cfg, err := testEnv.Start() + if err != nil { + fmt.Printf("failed to start testEnv: %v", err) + os.Exit(1) + } + + if err := v1alpha1.AddToScheme(testScheme); err != nil { + fmt.Printf("failed to register scheme: %v", err) + os.Exit(1) + } + + k8sClient, err = client.New(cfg, client.Options{Scheme: testScheme}) + if err != nil { + fmt.Printf("failed to setup a Kubernetes client: %v", err) + os.Exit(1) + } + + code := m.Run() + + err = testEnv.Stop() + if err != nil { + fmt.Printf("failed to stop testEnv: %v", err) + os.Exit(1) + } + + os.Exit(code) +} diff --git a/pkg/instrumentation/upgrade/upgrade_test.go b/pkg/instrumentation/upgrade/upgrade_test.go new file mode 100644 index 0000000000..5baa97d9a9 --- /dev/null +++ b/pkg/instrumentation/upgrade/upgrade_test.go @@ -0,0 +1,76 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package upgrade + +import ( + "context" + "strings" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" +) + +func TestUpgrade(t *testing.T) { + nsName := strings.ToLower(t.Name()) + err := k8sClient.Create(context.Background(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nsName, + }, + }) + require.NoError(t, err) + + inst := &v1alpha1.Instrumentation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-inst", + Namespace: nsName, + Annotations: map[string]string{ + v1alpha1.AnnotationDefaultAutoInstrumentationJava: "java:1", + }, + }, + Spec: v1alpha1.InstrumentationSpec{ + Sampler: v1alpha1.Sampler{ + Type: v1alpha1.ParentBasedAlwaysOff, + }, + }, + } + inst.Default() + assert.Equal(t, "java:1", inst.Spec.Java.Image) + err = k8sClient.Create(context.Background(), inst) + require.NoError(t, err) + + up := &InstrumentationUpgrade{ + Logger: logr.Discard(), + DefaultAutoInstrJava: "java:2", + Client: k8sClient, + } + err = up.ManagedInstances(context.Background()) + require.NoError(t, err) + + updated := v1alpha1.Instrumentation{} + err = k8sClient.Get(context.Background(), types.NamespacedName{ + Namespace: nsName, + Name: "my-inst", + }, &updated) + require.NoError(t, err) + assert.Equal(t, "java:2", updated.Annotations[v1alpha1.AnnotationDefaultAutoInstrumentationJava]) + assert.Equal(t, "java:2", updated.Spec.Java.Image) +} From 6dac6f2ab31f2daa759fba092d5c8c9cdc1f8611 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 18 Nov 2021 16:46:22 +0100 Subject: [PATCH 3/3] Fix Signed-off-by: Pavol Loffay --- .../manifests/opentelemetry-operator.clusterserviceversion.yaml | 2 -- config/rbac/role.yaml | 2 -- pkg/instrumentation/upgrade/upgrade.go | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index cbd2f63eaa..84517fb5c2 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -171,8 +171,6 @@ spec: resources: - instrumentations verbs: - - create - - delete - get - list - patch diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 0c131d382f..bba28bd159 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -106,8 +106,6 @@ rules: resources: - instrumentations verbs: - - create - - delete - get - list - patch diff --git a/pkg/instrumentation/upgrade/upgrade.go b/pkg/instrumentation/upgrade/upgrade.go index 402c40d4d6..e15aaab15d 100644 --- a/pkg/instrumentation/upgrade/upgrade.go +++ b/pkg/instrumentation/upgrade/upgrade.go @@ -31,7 +31,7 @@ type InstrumentationUpgrade struct { Client client.Client } -//+kubebuilder:rbac:groups=opentelemetry.io,resources=instrumentations,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=opentelemetry.io,resources=instrumentations,verbs=get;list;watch;update;patch // ManagedInstances upgrades managed instances by the opentelemetry-operator. func (u *InstrumentationUpgrade) ManagedInstances(ctx context.Context) error {