no-jira: Private domains in survey for PowerVS

[miyamotoh]

Adding private domains to the "Base Domain" survey, and when chosen, the publish: value in resultant install-config.yaml reflects the Internal publishing strategy.

Proposed here is to denote the private/internal ones with (Internal) like this in survey;

And selecting one of the Internal ones would result in publish: Internal (otherwise External);

Signed-off-by: Hiro Miyamoto miyamotoh@us.ibm.com

[AshwinHIBM]

https://github.com/miyamotoh/installer/blob/private-domains-in-survey/pkg/asset/installconfig/installconfig_test.go#L23
baseDomain := &baseDomain{BaseDomain: "test-domain"}

Other than that, lgtm. install-config.yaml shows the correct values for base domain and publish strategy and I can generate correct manifests successfully.

[openshift-ci]

@miyamotoh: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-e2e-aws-ovn-upgrade	1db371c	link	false	/test okd-e2e-aws-ovn-upgrade

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/test-infra repository. I understand the commands that are listed here.

[miyamotoh]

@r4f4 I'm seeking guidance here. This PR is proposing to modify the common baseDomain struct, as I didn't see any other way to pass the survey choice down further. Is that a bad idea, or can you suggest another way?

[r4f4]

This one I'll defer to @patrickdillon

[patrickdillon]

Typically we only allow the most basic configuration in the survey. So publish: Internal has only been supported through the install config.

If there's a good reason to support private clusters through the survey for PowerVS (i.e. it's a common use case), I think this would be fine. Otherwise, it's not really a problem that needs to be solved as it can just be handled through the installconfig.

[patrickdillon]

This could be ok.
This creates an informal dependency for the Publishing Strategy on BaseDomain. Typically such dependencies would be formalized in the asset graph.

PublishingStrategy is a field in the Install Config, which depends on the BaseDomain. So the dependency chain seems theoretically fine.

[patrickdillon]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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:

• OWNERS [patrickdillon]

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

[mjturek]

/lgtm

[openshift-ci-robot]

@miyamotoh: This pull request explicitly references no jira issue.

In response to this:

Adding private domains to the "Base Domain" survey, and when chosen, the publish: value in resultant install-config.yaml reflects the Internal publishing strategy.

Proposed here is to denote the private/internal ones with (Internal) like this in survey;

And selecting one of the Internal ones would result in publish: Internal (otherwise External);

Signed-off-by: Hiro Miyamoto miyamotoh@us.ibm.com

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.

[joepvd]

Rudely putting
/hold
wanting to get #8343 in first, and not wait for retests in case this one makes it earlier.

[r4f4]

/hold cancel

[miyamotoh]

/cherry-pick release-4.15

[openshift-cherrypick-robot]

@miyamotoh: #8303 failed to apply on top of branch "release-4.15":

Applying: Adding private DNS domains to survey for PowerVS
Using index info to reconstruct a base tree...
M	pkg/asset/installconfig/basedomain.go
M	pkg/asset/installconfig/installconfig.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/asset/installconfig/installconfig.go
Auto-merging pkg/asset/installconfig/basedomain.go
CONFLICT (content): Merge conflict in pkg/asset/installconfig/basedomain.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Adding private DNS domains to survey for PowerVS
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.15

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.