dynatrace-operator icon indicating copy to clipboard operation
dynatrace-operator copied to clipboard

Fixing conversion issues for v1beta2

Open 0sewa0 opened this issue 9 months ago • 0 comments

Description

This is all the changes I did in order to find the source of the super annoying error:

admission webhook "webhook.dynatrace.com" denied the request: unable to decode dynatrace.com/v1beta1, Kind=DynaKube into *dynakube.DynaKube

But in my search for the cause I also did some changes that I think are an improvement OR absolutely necessary.

Detailed changes:

  • The fix for the admission error:
    • It had nothing really to do with the conversion webhook, the problem was that the ValidatingWebhookConfiguration still was asking for v1beta1 even though the code wanted to work with v1beta2, and (I guess) the validation webhook wanted validate the v1beta1 dynakube BEFORE the conversion even happened, but failed, as it couldn't convert it on its own.
  • Moved the Hub to the v1beta2 version
    • If our code is working only with v1beta2 it makes no sense to store it in v1beta1 as that would lead to constant conversions.
      • I know we did it like that for v1alpha1 but its incorrect, so lets not repeat our mistake.
  • Updated the ConvertTo/From functions
    • Before my change, the ConvertTo/From function only cared about the conversion that I would consider "more complicated", but it didn't much care for the very annoying and monotonous ones. (like copying all the v1beta1.CloudNativeSpec to the 99% same v1beta2.CloudNativeSpec)
    • These super monotonous conversions need to be done, because when you are using the webhook conversion method, there is no help from kubernetes, its all our responsibility to do these small conversion.
  • Changed the new field DynatraceApiRequestThreshold to a simple int (from time.Duration)
    • This is to remain consistent with how the feature-flag worked (just an int value that is considered as minutes)
    • Context: When the feature-flag was implemented I wanted to use the time.Duration format (like 10m and such) but it was considered "too complicated" so we went with this "simple" approach

‼️ If you look at by commits it might be easier to understand ‼️ The 1000+ additions are the conversion logic and related tests (most lines are for the tests)

How can this be tested?

Apply 1 v1beta1 Dynakube, should work and move most fields (didn't fix the moving of the whole status) Apply 1 v1beta2 Dynakube, should just work, didn't touch the conversion logic of that, but didn't have a problem with it as of now

0sewa0 avatar May 15 '24 11:05 0sewa0