search

dice-cyfronet / atmosphere

Atmosphere cloud platform
MIT License
7 stars 3 forks source link

Remove google cloud and azure from supported technologies #282

Closed mkasztelnik closed 8 years ago

mkasztelnik commented 8 years ago

We are not able to test is anymore and existing azure and google cloud libraries are not compatible with fog 1.37 version.

@nowakowski, @tbartynski please take a look

tbartynski commented 8 years ago

:+1:

mkasztelnik commented 8 years ago

@nowakowski waiting for your opinion

nowakowski commented 8 years ago

Yes, but what about existing deployments (i.e. VPH-Share)? You need a migration or data curation task which will eliminate all ComputeSites of type azure or google, along with their VMTs etc. - otherwise the DB will be left in an inconsistent state.

mkasztelnik commented 8 years ago

As far as I know we don't have access to azure or google anymore. These compute sites are deactivated and not used anymore. You are right db need to be cleaned before deploying new atmo version.

mkasztelnik commented 8 years ago

Aaaa starting from yesterday gitub added reaction => instead of adding +1 in the comment please add reaction into issue description.

nowakowski commented 8 years ago

They're not deactivated on VPH because doing so breaks the version of atmosphere that's currently deployed on VPH (users are not able to retrieve AT lists when there exists at least one deactivated ComputeSite). Also, I cannot delete the Azure CS because of an improper callback cascade. I believe this PR should include a migration which deletes all "offending" records.

mkasztelnik commented 8 years ago

Hmmm we are not changing DB. You are right that deleting CS should also be fixed - you are free to create another PR which solves delete existing CS problem.

nowakowski commented 8 years ago

You are making a change in the model which may (and, in fact, will) invalidate the existing content of the DB. "Remember to clean up the DB when you deploy this change" does not look like a sound practice to me. I also don't see why data curation requires a separate PR when it's clearly part of the same problem. Sorry, no thumbs up from me. :)

mkasztelnik commented 8 years ago

@nowakowski this is a separate issue we need to solve before deploying to vph (but with deleting existing CS). I don't want to have one huge PR which is hard to review. Instead I prefer to have bunch of small PRs. And I encourage you to contribute and create separate PR which solve the issue with deleting existing CS.

nowakowski commented 8 years ago

@mkasztelnik Well, merge it if you want but I respectfully disagree with your view and I will not +1 this. You asked for my opinion, and my opinion is - this PR is incomplete. It is wrong to assume a "clean" database when writing PRs. We're talking about a system that's in production. If you change the model, you should include code which will adapt existing data to your new model. PR size is not an issue here. Btw., the CS deletion problem is already solved, although that fix is not yet deployed to VPH (which is why Azure and Google must currently remain active on VPH).

mkasztelnik commented 8 years ago

Good to hear that delete existing CS problem is already fixed (can you point me to PR which solves this issue? I was not aware about this fix).

Can we agree to following procedure:

If you want I can create rake task which will do safe delete of CSes, flavors, VMTs, VMs with not allowed cloud technology, but I'm not sure if this is needed since second step of proposed procedure should solve it (vph installation is the only one I'm aware with azure or google CS configured).

Is it ok with you?

nowakowski commented 8 years ago

Sorry; I was incorrect - the problem that's resolved is the CS inactivation issue (which was due to a fault in default optimizer - that was fixed when I rewrote the optimizer to work with tenants). The deletion bug is still there. :( OK on the plan.