danm
danm copied to clipboard
Update the domain for danm
What type of PR is this?
Uncomment only one, leave it on its own line:
bug cleanup design documentation failing-test feature
What does this PR give to us: Compatibility with the new versions of Kubernetes
Which issue(s) this PR fixes (in fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):
Fixes #
Special notes for your reviewer:
I modify the client created by client-gen
to add the Namespace to the Update function
Does this PR introduce a user-facing change?:
Now the new API is `danm.io/v1`, also the CRD is created using `apiextensions.k8s.io/v1` and not `apiextensions.k8s.io/v1beta1`
1: does your commit message meant to imply you consider the comments resolved? if yes: they are not. I really fail to understand why do you keep tinkering with the existing attribute structure you must keep everything as is. literally. go through the original YAML files attribute by attribute, and retain the exat same structure, with the exact same validation rules, without modifying them, or adding anything else to the spec if for some reason this cannot be done explicitly state the reason attribute-by-attribute why modifications are required
2: so if I understand the context of client go related changes the only manual change you made here is the "Namespace" addition to the TenantConfig client's "Create" and "Update" operation if I compare those with the code the same library generates for another namespaced API, for example for DanmNet we can see these Namespace additions are there by default: https://github.com/nokia/danm/blob/master/crd/client/clientset/versioned/typed/danm/v1/danmnet.go#L115 this makes me think the client generator code is actually good, and our instructions are bad! Take a look at the following instruction set for TenantConfig: https://github.com/nokia/danm/blob/master/crd/apis/danm/v1/types.go#L116 if you compare this to the designation of DanmNet you can see the order of the instructions is different. maybe that's an issue? but more importantly if you compare the TenantConfigList instruction set to DanmNetList: https://github.com/nokia/danm/blob/master/crd/apis/danm/v1/types.go#L132 you can see we basically told the client gen library to generate the list nonNameSpaced. in all the other cases (except ClusterNetworkList) this designation is not there TL;DR let's try to 1: switch the instruction order above TenantConfig struct to match the same order used in the case of other APIs 2: remove the "nonNameSpaced" designation from TenantConfigList so it matches the other APIs then let's regenerate the client code with these changes. I hope with these corrections the generated code will be okay and we don't need to manually fix it
@alejandrojnm looks good to me now! only thing left is to fix CI, and see if it works in a real environment
CI complains that your branch still has the manual modification in the TenantConfig client code, but with the new modifications you should be able to revert it, it should work with the baseline generated code. please test it out, and then we can merge
@alejandrojnm looks good to me! are you still finalizing or testing it, or can I go ahead and merge?
I will test all tomorrow to see if work as expected, I will let you know, in that way you can merge
@alejandrojnm any updates?
I will try to test this week, I have some tasks pending, sorry for the delay
Hi @Levovar we test this in kubernetes 1.24 and work, I will send all the test env when we test all, but is working 🎉