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.

Sign up for GitHub

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

Jump to bottom

Add huawei cloud controller #10198

Merged
merged 14 commits into from
Aug 25, 2023
Merged

Add huawei cloud controller #10198

merged 14 commits into from
Aug 25, 2023

Conversation

dibi-codes
Copy link
Contributor

What type of PR is this?
/kind feature

What this PR does / why we need it:
This PR adds the Huawei Cloud Controller which is used for Huawei compatbile cloud environments like Open Telekom Cloud. This is needed because these systems are based on openstack but do NOT support the octavia API. While the Neutron API isn't supported by the openstack provider anymore.

Does this PR introduce a user-facing change?:

Add the huawei cloud controller as external cloud controller

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 7, 2023
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 7, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @dabeck. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. labels Jun 7, 2023
@dibi-codes dibi-codes changed the title Feature/huawei cloud controller Add huawei cloud controller Jun 7, 2023
MrFreezeex
MrFreezeex reviewed Jun 8, 2023
View reviewed changes
inventory/sample/group_vars/all/huaweicloud.yml
## Credentials to authenticate against Keystone API
## All of them are required Per default these values will be
## read from the environment.
# external_huaweicloud_auth_url: "{{ lookup('env','OS_AUTH_URL') }}"
Copy link
Member

@MrFreezeex MrFreezeex Jun 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I see a lot of things related to OpenStack from a rapid search it seems to me that huawei cloud is somehow using OpenStack so are those really valid or is it just a copy paste "gone wrong"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is really close to openstack since huaweicloud is based on openstack BUT it is not fully compatible. Especially the loadbalancers are not offering Octavia Support which is a MUST since k8s 1.26 with the openstack provider. That's why huawei introduced it's own cloud provider as kubernetes-sig project.
But I get the point. Maybe we could use the existing OpenStack Provider and make the image field mutable so that one could use the huawei fork!?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the insight well I don't really mind just wanted to make sure it was intentional to leave the reference to Openstack :D

inventory/sample/group_vars/all/huaweicloud.yml
# external_huaweicloud_cloud: "{{ lookup('env','OS_CLOUD') }}"

## The repo and tag of the external Huawei Cloud Controller image
# external_huawei_cloud_controller_image_repo: "swr.ap-southeast-1.myhuaweicloud.com"
Copy link
Member

Choose a reason for hiding this comment

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

Is it the official way of pulling the image don't they have it published in a global registry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly yes: https://github.com/kubernetes-sigs/cloud-provider-huaweicloud/releases

roles/kubernetes-apps/external_cloud_controller/huaweicloud/OWNERS Outdated Show resolved Hide resolved
@yankay
Copy link
Member

yankay commented Jun 15, 2023

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 15, 2023
MrFreezeex
MrFreezeex approved these changes Jul 24, 2023
View reviewed changes
Copy link
Member

@MrFreezeex MrFreezeex left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! :D
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 24, 2023
@MrFreezeex
Copy link
Member

Hi Could you rebase your PR to fix the CI? 🙏

@dibi-codes dibi-codes force-pushed the feature/huawei-cloud-controller branch from 20a3ce1 to 15bafef Compare July 27, 2023 08:23
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 27, 2023
@MrFreezeex
Copy link
Member

Ah there is an error in ansible-lint now :(
roles/kubernetes-apps/external_cloud_controller/huaweicloud/tasks/main.yml:2: name[missing]: All tasks should be named..
You need to add a name to include_tasks and it should be good 👍

@MrFreezeex
Copy link
Member

MrFreezeex commented Jul 27, 2023 •
edited
Loading

Ah! well now your branch name doesn't play well with our testing scripts apparently, will send a PR for that 👍

@MrFreezeex MrFreezeex mentioned this pull request Jul 27, 2023
Merged
@dibi-codes dibi-codes mentioned this pull request Aug 24, 2023
Closed
@MrFreezeex
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2023
@yankay
Copy link
Member

yankay commented Aug 25, 2023

Thanks @dabeck
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dabeck, MrFreezeex, yankay

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2023
@k8s-ci-robot k8s-ci-robot merged commit e573a2f into kubernetes-sigs:master Aug 25, 2023
guytet pushed a commit to guytet/kubespray that referenced this pull request Oct 30, 2023
@dibi-codes
Add huawei cloud controller (kubernetes-sigs#10198)
621098a 
* Add huaweicloud as external cloud controller

* Add huaweicloud example config

* Rename AK,SK to ACCESS_KEY and SECRET_KEY

* Add reference to huaweicloud

* Fix variable naming

* Fix env var name

* Update example

* Fix variable naming

* Fix cloud_config path

* Add namespace for leader election

* Revert reviewers

* Delete OWNERS

Delete owners who are not responsible here.

* Fix build validation
pedro-peter pushed a commit to pedro-peter/kubespray that referenced this pull request May 8, 2024
@dibi-codes @pedromcpedro
Add huawei cloud controller (kubernetes-sigs#10198)
7227cb6 
* Add huaweicloud as external cloud controller

* Add huaweicloud example config

* Rename AK,SK to ACCESS_KEY and SECRET_KEY

* Add reference to huaweicloud

* Fix variable naming

* Fix env var name

* Update example

* Fix variable naming

* Fix cloud_config path

* Add namespace for leader election

* Revert reviewers

* Delete OWNERS

Delete owners who are not responsible here.

* Fix build validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@floryut floryut floryut left review comments

@MrFreezeex MrFreezeex MrFreezeex approved these changes

@liupeng0518 liupeng0518 Awaiting requested review from liupeng0518

Assignees

@MrFreezeex MrFreezeex

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dibi-codes @k8s-ci-robot @yankay @MrFreezeex @floryut