Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Big GLUE 2.1 rebase #99

Merged
merged 191 commits into from
Oct 16, 2018
Merged

Big GLUE 2.1 rebase #99

merged 191 commits into from
Oct 16, 2018

Conversation

gwarf
Copy link
Member

@gwarf gwarf commented Aug 29, 2018

Summary

This Work In Progress PR adds initial support for GLUE 2.1 Schema that includes improved cloud-specific information.

It's based on some quite old work done in the context of the INDIGO project (it includes work on keystone v3 support and other aspects by @alvarolopez).
It consists of GLUE 2.1-specific LDIF template files, but as the data structure between GLUE 2.0 and GLUE 2.1 differs vastly, it was needed to update the provider too.

  • Please note that
    • For the time being only the OpenStack provider was tested with the GLUE 2.1 output.
    • The GLUE 2.1 version was implemented is not the latest and greatest draft version
    • And furthemore GLUE 2.1 schema is not yet finalized (but it not likely to be requested massive change as it is being reviewed by the GLUE Working Group)
    • As this work has a long and complex history (keystone v3 support was added to the GLUE2.1 branch, then it was extracted to be put in the master branch, but due to some GLUE 2.1 specific features it had to be adapted) thinks will have to be tested carefully before goint to production, and side effects needs to be tested
    • Some test files (test_core.py and test_static.py) are still using a lot of @unittest.expectedFailure and should be fixed

To be done (or at lest considerder) before merge:

  • Update to latest GLUE 2.1 version
  • Validate GLUE 2.1 templates output
  • Validate that GLUE 2.0 templates will output the same thing as we the current version to keep backward compatibility for OpenStack
  • Validate that GLUE 2.0 templates will output the same thing as we the current version to keep backward compatibility for OpenNebula
  • Validate that GLUE 2.1 version works with OpenNebula (it quite unlikely that no code change will be needed)
  • Fix existing @unittest.expectedFailure tests for core.py and static.py providers
  • Check that documentation is complete

Related issues : #35, #87 and #88

And some emoji as requested by @brucellino: ⚠️ 😓 🚧 🕴
😄

@gwarf gwarf added this to the 1.0 milestone Aug 29, 2018
@gwarf gwarf self-assigned this Aug 29, 2018
@gwarf gwarf added the WIP Work In Progress label Aug 29, 2018
@gwarf gwarf requested a review from enolfc August 29, 2018 16:43
@coveralls
Copy link

coveralls commented Aug 29, 2018

Pull Request Test Coverage Report for Build 288

  • 175 of 308 (56.82%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.644%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cloud_info_provider/providers/init.py 17 20 85.0%
cloud_info_provider/providers/opennebula.py 17 31 54.84%
cloud_info_provider/providers/static.py 13 29 44.83%
cloud_info_provider/core.py 11 37 29.73%
cloud_info_provider/providers/ssl_utils.py 5 41 12.2%
cloud_info_provider/providers/openstack.py 48 86 55.81%
Totals Coverage Status
Change from base Build 279: 0.0%
Covered Lines: 1389
Relevant Lines: 1641

💛 - Coveralls

enolfc
enolfc previously requested changes Aug 30, 2018
Copy link
Contributor

@enolfc enolfc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

178 commits 🙄... no way to review this in a sane way. Overall LGTM.
I would propose to fix the pep8 error, and try to figure out what to do with the codeclimate (is it now worth it to reduce the cognitive complexity???) and then merge.

@@ -355,6 +397,56 @@ def get_images(self):
images[img_id] = aux_img
return images

@_rescope
def get_instances(self, os_project_name=None, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😎Are we really publishing this information? I guess it can be valuable for security teams to know if a given image is running

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

looking at the glue draft, I see there is no link between instances and images, so publishing the instances would not be really useful :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the GLUE 2.1 cloud model there is such a relation:
glue-2 1-cloud-computing-model

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking at the table with the description and if I'm not wrong it's missing there. I have added a comment in the spec draft

@brucellino
Copy link
Member

gwarf-wants-to-merge-179-commits-aint-nobody-got-time-fo-dat
Sorry, I couldn't resist.
I can't comment on the code, but I will take a look through the documentation and other files that may be included.

brucellino
brucellino previously approved these changes Aug 30, 2018
Copy link
Member

@brucellino brucellino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have nothing of value to add. I have at least read the PR though 🤷‍♂️

return obj_name[start:end]

@_rescope
def get_compute_endpoints(self, os_project_name=None, **kwargs):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alvarolopez this change (and the similar ones on get_templates and get_images are not coherent with the base class:
https://github.com/EGI-Foundation/cloud-info-provider/blob/d07d879094e631c26e0f051eaa9721f8fa7139a2/cloud_info/providers/__init__.py#L17
Is there any clean way to easily handle this?

@brucellino
Copy link
Member

@gwarf looks like bad credentials here are making the deploy fail in PR's. I think this is intended . https://community.egi.eu/t/travis-papercut-managing-secrets-with-travis/216/3

@gwarf
Copy link
Member Author

gwarf commented Oct 5, 2018

The deploy step are made for PR, but only for tags, but indeed there is still a problem with the credential in the master branch, but I'm lost, I've tried two times to change/update them and to recreate the tag but it still fails :/
The PR is failing due to a different reason: https://travis-ci.org/EGI-Foundation/cloud-info-provider/jobs/436170515#L1126

gwarf and others added 19 commits October 16, 2018 11:57
* Use project ids instead of project name
* Use Keystone URL instead of nova's
* openstack: load all available authentication plugins
* Load keystoneauth1 session arguments
* Nicer error information when templating
* Make it work with Keystone v2

Avoids having different endpoints for each of the supported
projects. OCCI endpoint still uses the OCCI URL to keep
compatibility.

Instead of only loading the v3password authentication plugin, lets load
all the available keystoneauth plugins so that the user is able to use
their preferred authentication method.

Fixes EGI-Federation#86

Small changes to play nicely with Keystone v2 so it works with
Keystone-VOMS authentication that it still the main one in the
production infra.
* Adapt endpoints information
* Fetch nova URL and version for Glue1.2 render
* Renamed cloud_info to cloud_info_provider

This bring some coherence to the package and should fix coveralls build
Avoid importing OS libs without try/except
OpenNebula support.

* Avoid failing if one endpoint is not present
  Useful if the site does not support OCCI
* Add missing getters to providers
* Refactor SSL code
  Put the code into the baseprovider so it can be used by OpenStack and
  OpenNebula providers easily. May be worth to put it as a separate class.
* Avoid project which is too OS centric
  'project' is OS specific and does not fit within the ONE naming. Also
  used os_project_id as the configuration variable to be explicit about
  using the id and not the name.
* Scope images and templates to the VO
* Set default membership in shares
  Assume it's equal to the VO name for convenience if not defined.
Fix core.py tests.
@gwarf gwarf force-pushed the big_glue_2.1_rebase branch from 4dc0ae5 to f0f5623 Compare October 16, 2018 09:58
@gwarf
Copy link
Member Author

gwarf commented Oct 16, 2018

So unless anyone is against it I will merge this once the codacy report will be available. (@brucellino ? @enolfc ? @alvarolopez ? Any remark?)

@enolfc enolfc self-requested a review October 16, 2018 10:34
Copy link
Contributor

@enolfc enolfc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Crazy To Me :)

@gwarf gwarf merged commit 69d5466 into EGI-Federation:master Oct 16, 2018
@gwarf gwarf mentioned this pull request Oct 16, 2018
@gwarf gwarf deleted the big_glue_2.1_rebase branch November 19, 2020 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants