Extend supported service endpoints #1637

cjschaef · 2024-02-28T20:50:51Z

Add additional services to endpoint override support for VPC related services.

What this PR does / why we need it: Expands the supported IBM Cloud Services that can have endpoint overrides.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Special notes for your reviewer:

/area provider/ibmcloud

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

Extend supported IBM Cloud Service endpoint overrides

linux-foundation-easycla · 2024-02-28T20:50:57Z

The committers listed above are authorized under a signed CLA.

✅ login: cjschaef / name: Christopher J Schaefer (e51c0f2)

k8s-ci-robot · 2024-02-28T20:51:00Z

Welcome @cjschaef!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-ibmcloud 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-ibmcloud has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-02-28T20:51:01Z

Hi @cjschaef. 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 · 2024-02-28T20:51:03Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cjschaef
Once this PR has been reviewed and has the lgtm label, please assign mkumatag for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

netlify · 2024-02-28T20:51:08Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Name	Link
🔨 Latest commit	`e51c0f2`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/65e0a48d79b25c0008933091
😎 Deploy Preview	https://deploy-preview-1637--kubernetes-sigs-cluster-api-ibmcloud.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mkumatag · 2024-02-29T01:11:14Z

/ok-to-test

mkumatag · 2024-02-29T14:37:49Z

/cc @Prajyot-Parab

Add additional services to endpoint override support for VPC related services.

jeffnowicki · 2024-03-01T02:47:40Z

Note: we're following OpenShift convention for service names being PascalCase. While we don't want to impose any specific consumer conventions, in this particular case it would ease our (VPC) OpenShift use of CAPI if we could follow similar conventions (where it makes sense). Let us know your thoughts and if you are open to refactoring the current names to PascalCase also.

mkumatag · 2024-03-01T04:39:27Z

Note: we're following OpenShift convention for service names being PascalCase. While we don't want to impose any specific consumer conventions, in this particular case it would ease our (VPC) OpenShift use of CAPI if we could follow similar conventions (where it makes sense). Let us know your thoughts and if you are open to refactoring the current names to PascalCase also.

I don't mind but just make sure we keep the backward compatibility as well, so that existing consumers shouldn't be suffering from this refactor. e.g: we can deprecate vpc in this release and introduce VPC and next release you can remove vpc from the code.

Karthik-K-N · 2024-03-05T05:28:26Z

docs/book/src/getting-started.md

@@ -47,7 +47,7 @@ it into a management cluster using `clusterctl`.
   > `${ServiceRegion1}:${ServiceID1}=${URL1},${ServiceID2}=${URL2};${ServiceRegion2}:${ServiceID1}=${URL1...}`.


-    Supported ServiceIDs include - `vpc, powervs, rc`
+    Supported ServiceIDs include - `CIS, COS, DNSServices, IAM, KeyProtect, ResourceManager, powervs, rc, vpc`


As of today, We dont user any of CIS, DNSServices, KeyProtect services. so should we add them now or add them when we start using these services later.

make sense, lets remove the entries which aren't used in this controller at the moment.

@Karthik-K-N is the ResourceManager duplicate of rc here?

@Karthik-K-N also the iam endpoint is being set via IBMCLOUD_AUTH_URL param, so do we really need the IAM service ID?

I think so its duplicate, if we want to use ResourceManager instead of rc, may be we can add support for both for a release and based on requirement we can deprecate later.

additional information... we have our (IBM Cloud VPC) set of required endpoint services for OCP defined here: https://github.com/openshift/installer/blob/release-4.15/data/data/install.openshift.io_installconfigs.yaml#L3450-L3492

we default to public endpoints for these required services, but provide the ability to override via install-config arguments (in order to support restricted network (airgap) install scenario)

that served as our basis for this PR

Thanks for the information, not all those service endpoints from openshift will be used by the CAPI controller, e.g: CIS will be used by the ingress operator(part of OCP Cluster Operartors) which is out of scope for this project.

We will add which are really required for the CAPI controllers to run e.g: COS, IAM, ResourceController VPC etc..

I can add them later, but they could all possibly be required (depending on the Network design) as the Infrastructure resources get created/referenced in VPC related enhancements.

Or we can wait on merging this until I have completed the necessary enhancements to CAPI for VPC Infrastructure, if that appears to be a better option.

Let's have an incremental approach, you can merge the code with bare minimum required changes, later will add those additional endpoints as in when needed.

I will wait until I have my first series of improvements to CAPI and VPCluster management then, since that is where I will start requiring the additional endpoints.

k8s-ci-robot · 2024-03-13T15:06:04Z

PR needs rebase.

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.

mkumatag · 2024-03-13T15:21:36Z

we don't need this anymore, got it merged part of #1648.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/provider/ibmcloud Issues or PRs related to ibmcloud provider labels Feb 28, 2024

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Feb 28, 2024

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 28, 2024

k8s-ci-robot requested review from jichenjc and wentao-zh February 28, 2024 20:51

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 28, 2024

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Feb 28, 2024

cjschaef marked this pull request as ready for review February 28, 2024 21:23

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 28, 2024

k8s-ci-robot requested a review from relyt0925 February 28, 2024 21:23

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 Feb 29, 2024

k8s-ci-robot requested a review from Prajyot-Parab February 29, 2024 14:37

Extend supported service endpoints

e51c0f2

Add additional services to endpoint override support for VPC related services.

cjschaef force-pushed the vpc_endpoint_expansion branch from 1efdfa6 to e51c0f2 Compare February 29, 2024 15:36

Karthik-K-N reviewed Mar 5, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 13, 2024

mkumatag closed this Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend supported service endpoints #1637

Extend supported service endpoints #1637

cjschaef commented Feb 28, 2024 •

edited

Loading

linux-foundation-easycla bot commented Feb 28, 2024 •

edited

Loading

k8s-ci-robot commented Feb 28, 2024

k8s-ci-robot commented Feb 28, 2024

k8s-ci-robot commented Feb 28, 2024

netlify bot commented Feb 28, 2024 •

edited

Loading

mkumatag commented Feb 29, 2024

mkumatag commented Feb 29, 2024

jeffnowicki commented Mar 1, 2024 •

edited

Loading

mkumatag commented Mar 1, 2024

Karthik-K-N Mar 5, 2024

mkumatag Mar 5, 2024

Prajyot-Parab Mar 5, 2024 •

edited

Loading

Prajyot-Parab Mar 5, 2024

Karthik-K-N Mar 5, 2024

jeffnowicki Mar 5, 2024 •

edited

Loading

mkumatag Mar 5, 2024

cjschaef Mar 5, 2024

mkumatag Mar 6, 2024

cjschaef Mar 7, 2024

k8s-ci-robot commented Mar 13, 2024

mkumatag commented Mar 13, 2024

Extend supported service endpoints #1637

Extend supported service endpoints #1637

Conversation

cjschaef commented Feb 28, 2024 • edited Loading

linux-foundation-easycla bot commented Feb 28, 2024 • edited Loading

k8s-ci-robot commented Feb 28, 2024

k8s-ci-robot commented Feb 28, 2024

k8s-ci-robot commented Feb 28, 2024

netlify bot commented Feb 28, 2024 • edited Loading

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

mkumatag commented Feb 29, 2024

mkumatag commented Feb 29, 2024

jeffnowicki commented Mar 1, 2024 • edited Loading

mkumatag commented Mar 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prajyot-Parab Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffnowicki Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 13, 2024

mkumatag commented Mar 13, 2024

cjschaef commented Feb 28, 2024 •

edited

Loading

linux-foundation-easycla bot commented Feb 28, 2024 •

edited

Loading

netlify bot commented Feb 28, 2024 •

edited

Loading

jeffnowicki commented Mar 1, 2024 •

edited

Loading

Prajyot-Parab Mar 5, 2024 •

edited

Loading

jeffnowicki Mar 5, 2024 •

edited

Loading