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

Introduce new version v1beta2 for DynaKube and add conversion logic #3092

Closed
wants to merge 24 commits into from

Conversation

waodim
Copy link
Contributor

@waodim waodim commented Apr 30, 2024

With this introduction and conversion logic of version v1beta2 for the DynaKube is done. Version v1alpha1 is removed for DynaKube and the new hubVersion is v1beta1. It is also the storageVersion

Summary of things that are added:

  • a new field metaDataEnrichment was added to the .spec of the Dynakube. it has an enabled field (enabled by default) and a namespace selector.
  • a new field dynatraceApiRequestsThreshold was added to the .spec of the Dynakube.
  • a new field secCompProfile was added to relevant oneAgent sections: hostMonitoring, classicFullStack, cloudNativeFullstack

Please have a look at the tags if they are correct and also check my wording please.

Corresponding JIRA tickets: K8S-9331 and K8S-9492

Description

How can this be tested?

Take your time to:

  • deploy operator
  • try to query dynakubes
  • test basic functionalities

@waodim waodim added the core Changes to core functionality of the Operator label Apr 30, 2024
@waodim waodim requested a review from a team as a code owner April 30, 2024 10:07
@waodim waodim marked this pull request as draft April 30, 2024 10:36
@waodim waodim changed the title Introduce new version v1beta2 for DynaKube and add conversion logic WIP: Introduce new version v1beta2 for DynaKube and add conversion logic Apr 30, 2024
@waodim waodim marked this pull request as ready for review May 2, 2024 08:05
@codecov-commenter
Copy link

codecov-commenter commented May 2, 2024

Codecov Report

Attention: Patch coverage is 48.53683% with 510 lines in your changes are missing coverage. Please review.

Project coverage is 58.09%. Comparing base (a54a20a) to head (dd478b5).
Report is 21 commits behind head on main.

Current head dd478b5 differs from pull request most recent head 91a3d9e

Please upload reports for the commit 91a3d9e to get more accurate results.

Files Patch % Lines
pkg/api/v1beta2/dynakube/zz_generated.deepcopy.go 0.00% 182 Missing ⚠️
pkg/api/v1beta2/dynakube/properties.go 43.47% 139 Missing and 4 partials ⚠️
pkg/api/v1beta2/dynakube/dynakube_conversion.go 55.76% 36 Missing and 10 partials ⚠️
pkg/api/v1beta2/dynakube/feature_flags.go 57.14% 36 Missing ⚠️
pkg/api/v1beta2/dynakube/proxy.go 0.00% 25 Missing ⚠️
pkg/api/v1beta2/dynakube/certs.go 0.00% 19 Missing ⚠️
pkg/api/v1beta2/dynakube/dynakube_status.go 0.00% 17 Missing ⚠️
...ntrollers/dynakube/oneagent/daemonset/daemonset.go 50.00% 10 Missing ⚠️
...kg/controllers/dynakube/dynatraceclient/builder.go 0.00% 8 Missing ⚠️
pkg/controllers/dynakube/conditions.go 60.00% 3 Missing and 3 partials ⚠️
... and 12 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3092      +/-   ##
==========================================
- Coverage   59.25%   58.09%   -1.17%     
==========================================
  Files         319      324       +5     
  Lines       17446    17891     +445     
==========================================
+ Hits        10338    10393      +55     
- Misses       5941     6319     +378     
- Partials     1167     1179      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@waodim waodim changed the title WIP: Introduce new version v1beta2 for DynaKube and add conversion logic Introduce new version v1beta2 for DynaKube and add conversion logic May 2, 2024
pkg/api/v1beta2/dynakube/feature_flags.go Outdated Show resolved Hide resolved
pkg/api/v1beta2/dynakube/feature_flags.go Outdated Show resolved Hide resolved
pkg/api/v1beta2/dynakube/feature_flags.go Outdated Show resolved Hide resolved
pkg/api/v1beta2/dynakube/dynakube_conversion.go Outdated Show resolved Hide resolved
pkg/api/v1beta2/dynakube/dynakube_conversion.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@chrismuellner chrismuellner left a comment

Choose a reason for hiding this comment

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

Just checking occurrences of v1beta1/v1alpha1 in the code base, I think we need to adapt a few more files:

  • PROJECT has v1alpha1 and v1beta1 only
  • troubleshoot package and command still use v1beta1
  • v1alpha1 is still used in a bunch of places (can we completely remove it?)

@waodim
Copy link
Contributor Author

waodim commented May 6, 2024

Just checking occurrences of v1beta1/v1alpha1 in the code base, I think we need to adapt a few more files:

* `PROJECT` has `v1alpha1` and `v1beta1` only

* `troubleshoot` package and command still use `v1beta1`

* `v1alpha1` is still used in a bunch of places (can we completely remove it?)
  • I updated PROJECT locally and will push soon
  • In my opinion troubleshoot using v1beta1 is okay as it is our hub version now, or am I wrong there?
  • I did not find usages of v1alpha1 version of the dynakube to be honest. When it comes to the whole v1alpha1 package we should not remove it in my opinion because of edgeconnect

@chrismuellner
Copy link
Collaborator

I would assume that all our logic should use the same, latest version of v1beta2.

I missed that the v1alpha1 package is required by edgeconnect, good point 😉

pkg/api/v1beta2/dynakube/dynakube_conversion.go Outdated Show resolved Hide resolved
pkg/api/v1beta2/dynakube/dynakube_conversion.go Outdated Show resolved Hide resolved
pkg/api/v1beta2/dynakube/dynakube_conversion.go Outdated Show resolved Hide resolved
@waodim waodim requested a review from 0sewa0 May 13, 2024 14:03
Copy link
Contributor

@0sewa0 0sewa0 left a comment

Choose a reason for hiding this comment

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

The conversion doesn't seem to work, when I create a v1beta1 DynaKube I get the following error:

Error from server: error when creating "./.vscode/deploy/dynakube_zib50933.yaml": admission webhook "webhook.dynatrace.com" denied the request: unable to decode dynatrace.com/v1beta1, Kind=DynaKube into *dynakube.DynaKube

Nothing in the logs.

I have been trying to fix it, introduced several changes but that did nothing.
The only relevant change is probably the missing version here, but that still didn't fix the issue

}

if src.Annotations[dynakube.AnnotationFeatureApiRequestThreshold] != "" {
duration, err := time.ParseDuration(src.Annotations[dynakube.AnnotationFeatureApiRequestThreshold])
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct as ParseDuration is actually smart and allows such things as 10h, but we only allow raw ints, that we use as seconds (I originally fought to use the actual ParseDuration, but it was deemed "to confusing" 🫤)

Copy link
Contributor

Choose a reason for hiding this comment

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

my attempts so far: 1990796

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So basically you switched hub version if I see that correctly?

Thank you I will have a look and hopefully fix it

Copy link
Contributor

@0sewa0 0sewa0 May 14, 2024

Choose a reason for hiding this comment

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

So basically you switched hub version

that was something I tried, it does not fix the problem

(I think its better to have the new version as the hub, if we are only using that in code, it should result in less conversions)

@0sewa0
Copy link
Contributor

0sewa0 commented May 22, 2024

Rebased PR #3188 is already merged

@0sewa0 0sewa0 closed this May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Changes to core functionality of the Operator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants