CORS-3709: GCP: Update Master pointer Ignition with API-Int IP #9085
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
sadasu
changed the title
|
/jira refresh |
|
@sadasu: No Jira issue is referenced in the title of this pull request. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
sadasu
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sadasu: This pull request references CORS-3709 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@sadasu: This pull request references CORS-3709 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
| return nil, fmt.Errorf("failed to unmarshal master ignition: %w", err) | ||
| } | ||
|
|
||
| ignitionHost := net.JoinHostPort(privateLBs[0], "22623") |
There was a problem hiding this comment.
| ignitionHost := net.JoinHostPort(privateLBs[0], "22623") | |
| if len(privateLBs) == 0 { | |
| return nil, fmt.Errorf("no private load balancer ip address found") | |
| } | |
| ignitionHost := net.JoinHostPort(privateLBs[0], "22623") |
| @@ -35,7 +41,8 @@ const ( | |||
| // EditIgnition attempts to edit the contents of the bootstrap ignition when the user has selected | |||
| // a custom DNS configuration. Find the public and private load balancer addresses and fill in the | |||
| // infrastructure file within the ignition struct. | |||
| func EditIgnition(ctx context.Context, in clusterapi.IgnitionInput) ([]byte, error) { | |||
| func EditIgnition(ctx context.Context, in clusterapi.IgnitionInput) ([]byte, []byte, error) { | |||
| // Why do we need another context here? | |||
There was a problem hiding this comment.
We don't this is going away I believe.
| @@ -35,7 +41,8 @@ const ( | |||
| // EditIgnition attempts to edit the contents of the bootstrap ignition when the user has selected | |||
| // a custom DNS configuration. Find the public and private load balancer addresses and fill in the | |||
| // infrastructure file within the ignition struct. | |||
| func EditIgnition(ctx context.Context, in clusterapi.IgnitionInput) ([]byte, error) { | |||
| func EditIgnition(ctx context.Context, in clusterapi.IgnitionInput) ([]byte, []byte, error) { | |||
There was a problem hiding this comment.
Is there any scenario the installer has where the bootstrap AND master don't need to both be edited? Wondering If this needs to be separated into two functions for EditBootstrapIgnition and EditControlPlaneIgnition.
There was a problem hiding this comment.
The control-plane pointer ignition is updated in its own method updatePointerIgnition().
I had considered the scenario that you mentioned. I am sure there might be times when only one Ignition needs to be updated. But, I am unable to come up with an example now.
These methods to manipulate the ignition are very specific to the exact changes needed for custom-dns. I think it is OK to design these just for this purpose without worrying about other scenarios where we might need Ignition editing functions that are completely generic and independent of each other.
There was a problem hiding this comment.
I am ok with this. I just thought we should consider it.
When userProvisionedDNS(custom-dns) is configured, the Master Pointer Ignition needs to be updated with the API-INT IP in place of the API-Int URL because at this stage in-cluster DNS is not running yet and the full Ignition needs to be downloaded.
When Master Pointer Ignition is updated with the API-Int IP address in place of the API-Int URL, update the Master Machine Configs within the Bootstrap Ignition with this new Master Pointer Ignition.
sadasu
force-pushed
the
sdasu-update-pointer-ignition
branch
from
5bbfb16
to
463285c
Compare
|
@sadasu: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
When userProvisionedDNS(custom-dns) is configured, the Master Pointer Ignition needs to be updated with the API-INT IP in place of the API-Int URL because at this stage in-cluster DNS is not running yet and the full Ignition needs to be downloaded.