EGI-Federation / cloud-info-provider

EGI Cloud Information System Provider
Apache License 2.0
3 stars 15 forks source link

Merging work from DEEP-Hybrid-DataCloud #214

Closed orviz closed 1 year ago

orviz commented 3 years ago

Summary

This massive PR is meant to contribute with the work done throughout the DEEP-Hybrid-DataCloud project, mainly:

Pending work:

The PR is created as draft PR in order to finish some pending work (more probably to be added):

:gift: :gift: for reviewers

gwarf commented 3 years ago

Thanks @orviz, but seeing all those changes in a single PR is quire frightening (and even more seeing there are pending things), quite a lot of change to review at once, especially for the stuff we are not using. Would it be feasible for you to break this PR in multiple ones that we could review, discuss and merge independently?

orviz commented 3 years ago

@enolfc @gwarf I understand, but it is not trivial thing to rescue only those commits that are specific of each new feature provided. This was evolving throughout the lifespan of the project and I could easily bypass something while doing this task.

The vast majority of the features included in this PR are currently leveraged by CMDB only. Thus, it is just a matter to be extra careful with the modifications in the OpenStack provider that could affect the GLUE21. Once having that covered, the rest of modifications (e.g. new providers) should only affect CMDB, which has been extensively tested (and leveraged by the INDIGO Orchestrator) by the sites from DEEP.

enolfc commented 3 years ago

Let's give it a try then. We may think of splitting providers/formatters into different repos and packages in the future to simplify management and allow them to evolve independently.

orviz commented 3 years ago

Regarding the isolation of the providers/formatters, we had discussed this in the past within DEEP, eventually did it with the Amazon provider [1]. There are pros and cons about this approach (there is also the possibility to group them in a single package: one pkg for providers, one for formatters). Whatever you decide it's ok for us.

[1] https://github.com/indigo-dc/cip-plugin-cmdb-publisher

orviz commented 3 years ago

Regarding the isolation of the providers/formatters, we had discussed this in the past within DEEP, eventually did it with the Amazon provider [1]. There are pros and cons about this approach (there is also the possibility to group them in a single package: one pkg for providers, one for formatters). Whatever you decide it's ok for us.

[1] https://github.com/indigo-dc/cip-plugin-cmdb-publisher

gwarf commented 3 years ago

Regarding the isolation of the providers/formatters, we had discussed this in the past within DEEP, eventually did it with the Amazon provider [1]. There are pros and cons about this approach (there is also the possibility to group them in a single package: one pkg for providers, one for formatters). Whatever you decide it's ok for us.

[1] https://github.com/indigo-dc/cip-plugin-cmdb-publisher

I guess you meant https://github.com/indigo-dc/cip-plugin-aws-provider

I'm curious about pro and cons, can you elaborate?

Please also address the points reported to the linter before marking this as ready for review.

orviz commented 3 years ago

Regarding the isolation of the providers/formatters, we had discussed this in the past within DEEP, eventually did it with the Amazon provider [1]. There are pros and cons about this approach (there is also the possibility to group them in a single package: one pkg for providers, one for formatters). Whatever you decide it's ok for us. [1] https://github.com/indigo-dc/cip-plugin-cmdb-publisher

I guess you meant https://github.com/indigo-dc/cip-plugin-aws-provider

you're right

I'm curious about pro and cons, can you elaborate?

I guess it all comes down to the release management. Isolation is good for not having a monolithic product, so If you just modify a single provider or formatter, you don't have to create a new CIP release. But, also in this case, if you modify the core, you may end up having to make not only the CIP release, but also one release per each isolated package (which wouldn't happen in the monolithic approach). Of course you can always use old CIP versions as dependencies in those packages..

Please also address the points reported to the linter before marking this as ready for review.

yes sure will do

enolfc commented 2 years ago

@orviz I think we should take a decision on how to move on with this one. There are useful changes here and I'd like to avoid having diverging forks but 290 commits are quite hard to manage.

I will try to do a rebase and squash changes to something smaller

enolfc commented 1 year ago

what should we do with this one? @gwarf @orviz

enolfc commented 1 year ago

Closing in favor of #237