search

nokia / danm

TelCo grade network management in a Kubernetes cluster
BSD 3-Clause "New" or "Revised" License
373 stars 81 forks source link

Updated the API domain to avoid problems with the last version of Kubernetes #268

Closed alejandrojnm closed 2 years ago

alejandrojnm commented 2 years ago

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:

Which issue(s) this PR fixes (in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes #

fixes #242

Special notes for your reviewer:

I updated the API domain to avoid problems with the last version of Kubernetes, I already tested in a cluster and work as expected with the new domain danm.io

Does this PR introduce a user-facing change?:

NONE
Levovar commented 2 years ago

this in itself is good, (looks like Travis is back to working order, yaay) but do note we have another repository hosting some utility functions which also somewhat depend on the correct API path: https://github.com/nokia/danm-utils Particularly Cleaner is strongly proposed to be always deployed in production alongside the DANM meta, so in case you made this change because you use DANM in prod take note, you might want to extend teh scope to at least Cleaner as well

Levovar commented 2 years ago

also note that this change in itself will not be enough to deploy DANM in newer K8s environments. DANM CRDs also need to be upgraded to GA V1 CRD API from v1beta1 the fix in this PR definitely solves one dependency of this upgrade, but if I recall correctly other GA API restrictions might also need fixing

alejandrojnm commented 2 years ago

Hi @Levovar, yes we know we need to update the existing DANM network to the new API, so we think we can use the danm-utils to do this, what did you think ??

Levovar commented 2 years ago

not exactly sure what is the correlation between the two topics, in my mind they are separate Issue1: the DANM API group name needs to be updated in danm-utils repo as well Issue2: the CRD API version in the DANM CRD templates need to be updated in both repos (was not part of this review) from v1beta to v1. Referring to these: https://github.com/nokia/danm/blob/master/integration/crds/production/ClusterNetwork.yaml#L1 then ofc tested that the current CRD body conforms to the V1 K8s CRD API (I don't think so), and also test the current client code works well with V1 CRDs too

BTW I just noticed you opened your pull request against my branch, but not against master. I guess we still need a PR to rebase the changes to master