Skip to content

Add AlibabaCloud Operator#15160

Merged
aanm merged 3 commits intocilium:masterfrom
l1b0k:alibaba
Apr 5, 2021
Merged

Add AlibabaCloud Operator#15160
aanm merged 3 commits intocilium:masterfrom
l1b0k:alibaba

Conversation

@l1b0k
Copy link
Copy Markdown
Contributor

@l1b0k l1b0k commented Mar 2, 2021

Hi, maintainers , this pr provide an AlibabaCloud Operator.
This allow Cilium deployments running in the AlibabaCloud and performs IP allocation based on IPs of ENI.

@l1b0k l1b0k requested a review from a team March 2, 2021 05:07
@l1b0k l1b0k requested a review from a team as a code owner March 2, 2021 05:07
@l1b0k l1b0k requested a review from a team March 2, 2021 05:07
@l1b0k l1b0k requested review from a team as code owners March 2, 2021 05:07
@l1b0k l1b0k requested review from a team March 2, 2021 05:07
@l1b0k l1b0k requested a review from a team as a code owner March 2, 2021 05:07
@l1b0k l1b0k requested a review from a team March 2, 2021 05:07
@l1b0k l1b0k requested review from a team as code owners March 2, 2021 05:07
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 2, 2021
@l1b0k l1b0k requested review from jibi, kkourt and nathanjsweet March 2, 2021 05:07
@l1b0k l1b0k requested a review from qmonnet March 2, 2021 05:07
@l1b0k l1b0k requested a review from gandro March 2, 2021 05:07
@l1b0k l1b0k requested a review from nebril March 2, 2021 05:07
@l1b0k l1b0k requested a review from tklauser March 2, 2021 05:07
@aanm aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Mar 2, 2021
@christarazi
Copy link
Copy Markdown
Member

@aanm Yes I think that makes sense, as we already do for all other cloud providers.

@joestringer
Copy link
Copy Markdown
Member

joestringer commented Mar 29, 2021

Just to synchronize with the discussion we had during the 2021-03-29 Cilium community meeting, we discussed that there is some degree of overlap between the common portions of the EKS, AKS and AlibabaCloud operator code. As a medium to long-term goal, we would all like to improve code reuse between these implementations where it makes sense, to minimize ongoing maintenance burden (and minimize the likelihoood of needing to fix a bug multiple times across the implementations). That said, it doesn't seem reasonable to place that entire burden of addressing tech debt onto @l1b0k.

We discussed that we should narrow down the remaining review feedback and make sure it's addressed, then when we're happy with the implementation, merge it and mark it as a "beta" feature for the initial v1.10 release. Follow up items after merging this PR would be to investigate any documentation (and/or Cilium CLI implementations), plus consider how regression testing will work for this new feature. During v1.11 release cycle we can further explore these questions and with more user feedback, evaluate marking the feature as stable in the documentation.

Copy link
Copy Markdown
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

@l1b0k thanks for the changes, since we are about to add the alibaba operator image, please also make appropariate changes in .github/workflows/images-legacy-releases.yaml, .github/workflows/images-legacy.yaml
and .github/workflows/images-legacy-hotfix-releases.yaml

@aanm
Copy link
Copy Markdown
Member

aanm commented Mar 31, 2021

The docker image repository for operator-alibabacloud{-ci,-dev} is created.

@aanm aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Mar 31, 2021
@kkourt kkourt removed their assignment Mar 31, 2021
l1b0k added 2 commits April 1, 2021 08:29
Signed-off-by: l1b0k <libokang.dev@gmail.com>
Signed-off-by: l1b0k <libokang.dev@gmail.com>
Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

New GitHub workflow bits look trivially correct to me, @aanm did you want to take another look?

@christarazi
Copy link
Copy Markdown
Member

test-me-please

@aanm
Copy link
Copy Markdown
Member

aanm commented Apr 2, 2021

@l1b0k there are some real failures detected by the CI here: https://github.com/cilium/cilium/pull/15160/checks?check_run_id=2250623707

Copy link
Copy Markdown
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

LGTM thanks. Only thing left is to address the issues caught by CI and this PR should be good to go.

@qmonnet
Copy link
Copy Markdown
Member

qmonnet commented Apr 5, 2021

test-me-please

The AlibabaCloud allocator is specific to Cilium deployments running in the AlibabaCloud and performs IP allocation based on IPs of ENI.

More for ENI https://www.alibabacloud.com/help/doc-detail/58496.htm

Signed-off-by: l1b0k <libokang.dev@gmail.com>
@qmonnet
Copy link
Copy Markdown
Member

qmonnet commented Apr 5, 2021

test-me-please

@aanm
Copy link
Copy Markdown
Member

aanm commented Apr 5, 2021

closing and re-opening for travis to kick in

@aanm
Copy link
Copy Markdown
Member

aanm commented Apr 5, 2021

Travis is not collaborating... If it breaks on master we can always revert.

Thank you for this contribution @l1b0k! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/major This PR introduces major new functionality to Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.