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

WIP - Caasp #52

Merged
merged 1 commit into from
May 17, 2020
Merged

WIP - Caasp #52

merged 1 commit into from
May 17, 2020

Conversation

brunoleon
Copy link
Collaborator

Adding CaaSP support

Copy link
Contributor

@jhesketh jhesketh left a comment

Choose a reason for hiding this comment

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

This is looking really good, thanks!

Just a few comments/nits. Let me know if you have any questions.

tests/lib/hardware/hardware_base.py Outdated Show resolved Hide resolved
tests/lib/hardware/hardware_base.py Outdated Show resolved Hide resolved
tests/lib/hardware/openstack_libcloud.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
@jhesketh
Copy link
Contributor

jhesketh commented May 7, 2020

By the way, are you still configuring a loadbalancer somewhere?

Copy link
Contributor

@toabctl toabctl left a comment

Choose a reason for hiding this comment

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

  • tests/lib/kubernetes.py → tests/lib/k8s/vanilla.py : I guess you need to rebase ? kubernetes.py is long gone
  • Please make useful commit messages and squash the whole thing

tests/assets/ansible/roles/basepackages/tasks/Suse.yaml Outdated Show resolved Hide resolved
tests/lib/ansible_helper.py Outdated Show resolved Hide resolved
tests/lib/hardware/hardware_base.py Outdated Show resolved Hide resolved
tests/lib/hardware/hardware_base.py Outdated Show resolved Hide resolved
tests/lib/hardware/hardware_base.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
@brunoleon
Copy link
Collaborator Author

By the way, are you still configuring a loadbalancer somewhere?

As we discovered the lack of support in libcloud.
The only solution I found would be using neutron-cli (or octavia but not support in ECP right now)

However, at first I just disregarded the LB completely and use the masters directly. This is no long term solution. We need to discuss it.

Copy link
Contributor

@toabctl toabctl left a comment

Choose a reason for hiding this comment

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

linter is failing

@brunoleon brunoleon force-pushed the caasp branch 9 times, most recently from 7e82c82 to bbef77c Compare May 14, 2020 10:54
Copy link
Contributor

@toabctl toabctl left a comment

Choose a reason for hiding this comment

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

  • please log the original error message when you reraise an exception. Otherwise we lose information for debugging
  • please don't rename files when not needed. This makes the whole review just more complicated so keep the kubernetes dir for this and rename it in another commit if you think it's needed.

tests/lib/ansible_helper.py Show resolved Hide resolved
tests/lib/hardware/hardware_base.py Outdated Show resolved Hide resolved
tests/lib/hardware/hardware_base.py Outdated Show resolved Hide resolved
tests/assets/ansible/roles/caasp/tasks/main.yaml Outdated Show resolved Hide resolved
@@ -24,6 +24,7 @@ class NodeBase(ABC):
"""
def __init__(self, name: str, private_key: str):
self.name = name
self.dnsname = self.name.replace('_', '-')
Copy link
Contributor

Choose a reason for hiding this comment

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

Still not resolved or answered

tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
tests/lib/k8s/caasp.py Outdated Show resolved Hide resolved
@brunoleon brunoleon force-pushed the caasp branch 4 times, most recently from 2cdc545 to 531623d Compare May 15, 2020 08:22
@brunoleon brunoleon force-pushed the caasp branch 5 times, most recently from 5ca0217 to cf4ad37 Compare May 15, 2020 10:27
Caasp deployment is making use of skuba deployment tool.
This requires adding support for an ssh-agent
Ideally would need a loadbalancer too but this is not
implemented yet due to libcloud provider not supporting
LB on openstack.
Use "raw" (i.e yaml) ansible playbook. The code is written
so that adding another distro support should be pretty easy.

Renames kubernetes folder to k8s which is shorter
and widely used
@brunoleon brunoleon requested a review from jhesketh May 15, 2020 12:58
@toabctl toabctl merged commit e9c302d into SUSE:master May 17, 2020
@brunoleon brunoleon mentioned this pull request May 18, 2020
@brunoleon
Copy link
Collaborator Author

Fixes #19

@brunoleon
Copy link
Collaborator Author

resolves #19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants