Skip to content

API changes for BYO AWS infra - #38

Merged
openshift-merge-robot merged 2 commits into
openshift:mainfrom
csrwng:byo_infra_api_changes
Feb 18, 2021
Merged

API changes for BYO AWS infra#38
openshift-merge-robot merged 2 commits into
openshift:mainfrom
csrwng:byo_infra_api_changes

Conversation

@csrwng

@csrwng csrwng commented Feb 15, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Proposal for changes to the API to support BYO infra for hypershift on AWS

So we need infra information to satisfy these requirements:

  1. Know where to place the machines, with the proper subnet, instance profile, and security groups.
    • Question for this ... do we always want to have a default nodepool generated from the HostedCluster? or are we going to eventually require you to create those resources separately?
  2. Create a cloud configuration file so that the in-tree AWS provider in the KCM can manage load balancers, pvs.
  3. Setup the openvpn config so that machine IPs can be routed through the VPN.

@openshift-ci-robot

openshift-ci-robot commented Feb 15, 2021

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csrwng

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

The pull request process is described here

Details 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 15, 2021
@csrwng

csrwng commented Feb 15, 2021

Copy link
Copy Markdown
Contributor Author

/cc @enxebre @derekwaynecarr

enxebre
enxebre reviewed Feb 16, 2021
View reviewed changes
Comment thread docs/api.md Outdated
derekwaynecarr
derekwaynecarr reviewed Feb 16, 2021
View reviewed changes
Comment thread docs/api.md Outdated
csrwng
csrwng commented Feb 16, 2021
View reviewed changes
Comment thread docs/api.md Outdated
csrwng
csrwng commented Feb 16, 2021
View reviewed changes
Comment thread docs/api.md Outdated
@csrwng
csrwng force-pushed the byo_infra_api_changes branch from c941a4a to 20143d4 Compare February 16, 2021 15:58
@csrwng

csrwng commented Feb 16, 2021

Copy link
Copy Markdown
Contributor Author

Current set of comments addressed

@csrwng
csrwng force-pushed the byo_infra_api_changes branch from 20143d4 to 3eccaac Compare February 16, 2021 17:41
@csrwng
csrwng force-pushed the byo_infra_api_changes branch from 3eccaac to 9455f23 Compare February 16, 2021 17:48
enxebre
enxebre reviewed Feb 17, 2021
View reviewed changes
Comment thread docs/api.md
Platform PlatformSpec `json:"platform"`

// InfraID is used to identify the cluster in cloud platforms
InfraID string `json:"infraID,omitempty"`

@enxebre enxebre Feb 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you remind me why we need this exposed in the spec?

@csrwng csrwng Feb 17, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@enxebre originally I had this in the status because it would be auto-generated by the hosted cluster controller, then I remembered that at some point we will likely need a way to restore a HostedCluster cr for a cluster that already exists (for DR). If not included in the spec, we won't be able to do this.

@enxebre enxebre Feb 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make sense, though may be wait to bake into the API until we have the story for DR fleshed out to avoid confusion

@enxebre

enxebre commented Feb 18, 2021

Copy link
Copy Markdown
Member

lgtm

@sjenning

sjenning commented Feb 18, 2021

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2021
@openshift-merge-robot
openshift-merge-robot merged commit f71b3c9 into openshift:main Feb 18, 2021
@csrwng csrwng mentioned this pull request Feb 24, 2021
Merged
4 tasks
zanetworker pushed a commit to zanetworker/hypershift that referenced this pull request Sep 28, 2021
@ironcladlou
Merge pull request openshift#38 from csrwng/fix_kcm
c5530da 
Move kube controller manager arguments to command line
@csrwng
csrwng deleted the byo_infra_api_changes branch June 14, 2022 19:12
@hypershift-jira-solve-ci hypershift-jira-solve-ci Bot mentioned this pull request Jul 9, 2026
Merged
4 tasks
hypershift-jira-solve-ci Bot pushed a commit to hypershift-community/hypershift that referenced this pull request Jul 9, 2026
OpenShift CI Bot
fix(operator): move router component after route-creating components
e3e689f 
The router's adaptConfig function lists existing Route objects to
generate the HAProxy ConfigMap. When the router was registered early in
the component list (position openshift#8), components that create Routes —
ignition-server (openshift#35) and metrics-proxy (openshift#38) — had not yet run during
the first reconcile pass. This caused the initial HAProxy config to miss
ignition and metrics-proxy backends, routing ignition requests to the
KAS default backend and failing NodePool ignition.

Move the router component to after metricsproxyv2 (the last
route-creating component) so that all Routes exist when the router
config is generated. This reduces the race window between router config
generation and route creation, though the informer cache update from
watch events is still async so the ordering is not a hard guarantee.

Signed-off-by: OpenShift CI Bot <ci-bot@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@enxebre enxebre enxebre left review comments
@derekwaynecarr derekwaynecarr derekwaynecarr left review comments
@ironcladlou ironcladlou Awaiting requested review from ironcladlou

Assignees

@sjenning sjenning

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@csrwng @openshift-ci-robot @enxebre @sjenning @ironcladlou @derekwaynecarr @openshift-merge-robot