MULTIARCH-2590: PowerVS create service instance #7695
Conversation
|
@hamzy: This pull request references MULTIARCH-2590 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.15.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 kubernetes/test-infra repository. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
hamzy
changed the title
|
/test ci/prow/govet |
|
@hamzy: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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 kubernetes/test-infra repository. |
|
/test govet |
hamzy
force-pushed
the
PowerVS-create-service-instance
branch
2 times, most recently
from
8134f94
to
9f11466
Compare
|
/retest-required |
|
/test tf-fmt |
hamzy
force-pushed
the
PowerVS-create-service-instance
branch
from
9f11466
to
dfe7dab
Compare
hamzy
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/assign @r4f4 |
We are moving away from using a predefined service instance (serviceInstanceID). We are now creating it in Terraform, so we cannot check for space in the pool, since we do not know the GUID yet.
Currently, a user has to create a Power IAAS service instance before creating a cluster. It will then be put into install-config.yaml in the serviceInstanceID field. Now, this field will be removed and the IPI installer will create the service instance itself.
Remove the requirement that the user creates a service instance before creating a cluster.
Since PVSNetworkName is associated with an existing Power IAAS service instance, and we are removing support for existing service instances, therefore we are removing it.
hamzy
force-pushed
the
PowerVS-create-service-instance
branch
from
dfe7dab
to
8261f9e
Compare
| return "", fmt.Errorf("failed to get instance: %w", err) | ||
| } | ||
| if response != nil && response.StatusCode == gohttp.StatusNotFound || response.StatusCode == gohttp.StatusInternalServerError { | ||
| return "", fmt.Errorf("failed to get instance, response is: %v", response) |
There was a problem hiding this comment.
There is a chance that you'll fail to get some resource instances but you can still find the one you're looking for. So you can increase the chances of success by saving the last error you saw but keep going through the resources. Then, at the end of the loop, you can return the last error found. Of course it's OK If you prefer to fail right away.
There was a problem hiding this comment.
Hrm. Destroy code is written to fail at the first sign of trouble. I do agree in this scenario, that ignoring an unrelated and very transitory hiccup while searching for the correct service instance could make this code more resilient. In the long run, with PowerVS, a problem found usually indicates more problems down the road.
There was a problem hiding this comment.
@mjturek We should have a TODO to mention in the documentation that destroy code will fail at the first sign of trouble. And that the customer should open a support ticket and try again after support responds.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: r4f4 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:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/lgtm |
openshift-merge-bot
bot
merged commit f60ebb0
into
openshift:master
Currently, a user has to create a Power IAAS service instance before creating a cluster. It will then be put into install-config.yaml in the serviceInstanceID field. Now, this field will be removed and the IPI installer will create the service instance itself.