Skip to content

Conversation

@jeffnowicki
Copy link
Contributor

The only supported 'existing resource' for 4.10 is a resource group. The additional existing resource options for vpc and subnets will be a future release consideration. Those options will be removed as options at this time. How soon those options can be supported will be a prioritization activity based on other considerations and available resources.

@openshift-ci openshift-ci bot requested review from kirankt and rna-afk January 24, 2022 00:34
@jeffnowicki jeffnowicki changed the title Bug 2042036 : remove options for existing resources which are currently not supported Bug 2042036: remove options for existing resources which are currently not supported Jan 24, 2022
@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jan 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 24, 2022

@jeffnowicki: This pull request references Bugzilla bug 2042036, which is invalid:

  • expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 2042036: remove options for existing resources which are currently not supported

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.

@pamoedom
Copy link
Contributor

/bugzilla refresh

@openshift-ci openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jan 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 24, 2022

@pamoedom: This pull request references Bugzilla bug 2042036, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @pamoedom

Details

In response to this:

/bugzilla refresh

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 openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jan 24, 2022
@openshift-ci openshift-ci bot requested a review from pamoedom January 24, 2022 09:33
Copy link
Contributor

@kirankt kirankt left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2022
@hasueki
Copy link
Contributor

hasueki commented Jan 24, 2022

@jeffnowicki IIRC, the contents for data/data/install.openshift.io_installconfigs.yaml are auto-gen'd. I think we should remove the definitions from https://github.com/openshift/installer/blob/master/pkg/types/ibmcloud/platform.go#L4, and then generate the above file.

@jeffnowicki
Copy link
Contributor Author

Thanks Hide... I'll correct the fix.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2022
@jeffnowicki
Copy link
Contributor Author

I still need to gen and commit install.openshift.io_installconfigs.yaml

@jeffnowicki
Copy link
Contributor Author

/retest-required

@kirankt
Copy link
Contributor

kirankt commented Jan 24, 2022

@jeffnowicki. These removed options are being checked during validation. Please remove references there as well. Thanks.
https://github.com/openshift/installer/blob/master/pkg/types/ibmcloud/validation/platform.go#L56-L64

@jeffnowicki
Copy link
Contributor Author

@kirankt Thank you for the review/comments. I updated validation code per your comment.

@pamoedom
Copy link
Contributor

Hi @jeffnowicki, with your latest changes I was able to locally compile the installer and the tests are successful, the options are no longer present nor accepted, thanks:

$ ./openshift-install-local version
./openshift-install-local unreleased-master-5524-g55a9f62e2693962bd4340917efded7407570d95a
built from commit 55a9f62e2693962bd4340917efded7407570d95a
release image registry.ci.openshift.org/origin/release:4.10
release architecture amd64

$ ./openshift-install-local explain installconfig.platform.ibmcloud
KIND:     InstallConfig
VERSION:  v1

RESOURCE: <object>
  IBMCloud is the configuration used when installing on IBM Cloud.

FIELDS:
    defaultMachinePlatform <object>
      DefaultMachinePlatform is the default configuration used when installing on IBM Cloud for machine pools which do not define their own platform configuration.

    region <string> -required-
      Region specifies the IBM Cloud region where the cluster will be created.

    resourceGroupName <string>
      ResourceGroupName is the name of an already existing resource group where the cluster should be installed. This resource group should only be used for this specific cluster and the cluster components will assume ownership of all resources in the resource group. Destroying the cluster using installer will delete this resource group. 
 This resource group must be empty with no other resources when trying to use it for creating a cluster. If empty, a new resource group will be created for the cluster.

$ ./openshift-install-local create manifests --dir test52/
FATAL failed to fetch Master Machines: failed to load asset "Install Config": failed to unmarshal install-config.yaml: failed to parse first occurence of unknown field: error unmarshaling JSON: while decoding JSON: json: unknown field "subnets"

$ ./openshift-install-local create manifests --dir test52/
FATAL failed to fetch Master Machines: failed to load asset "Install Config": failed to unmarshal install-config.yaml: failed to parse first occurence of unknown field: error unmarshaling JSON: while decoding JSON: json: unknown field "vpc"

$ ./openshift-install-local create manifests --dir test52/
FATAL failed to fetch Master Machines: failed to load asset "Install Config": failed to unmarshal install-config.yaml: failed to parse first occurence of unknown field: error unmarshaling JSON: while decoding JSON: json: unknown field "vpcResourceGroupName"

Regards.

@jeffnowicki
Copy link
Contributor Author

@kirankt would you review/add labels? Thank you!

@jstuever
Copy link
Contributor

/assign

@jstuever
Copy link
Contributor

/approve
I'll leave the lgtm for kirankt

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2022
Copy link
Contributor

@kirankt kirankt left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 27, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jstuever, kirankt, pamoedom

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-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@jstuever
Copy link
Contributor

/retest

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

10 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 28, 2022

@jeffnowicki: The following tests 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/e2e-aws-single-node 55a9f62 link false /test e2e-aws-single-node
ci/prow/e2e-aws-workers-rhel7 55a9f62 link false /test e2e-aws-workers-rhel7
ci/prow/e2e-ibmcloud 55a9f62 link false /test e2e-ibmcloud
ci/prow/okd-e2e-aws 55a9f62 link false /test okd-e2e-aws
ci/prow/e2e-crc 55a9f62 link false /test e2e-crc
ci/prow/e2e-aws-workers-rhel8 55a9f62 link false /test e2e-aws-workers-rhel8
ci/prow/e2e-alibaba 55a9f62 link false /test e2e-alibaba
ci/prow/e2e-azure-upi 55a9f62 link false /test e2e-azure-upi
ci/prow/okd-e2e-aws-upgrade 55a9f62 link false /test okd-e2e-aws-upgrade

Full PR test history. Your PR dashboard.

Details

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.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 7062f2b into openshift:master Jan 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 28, 2022

@jeffnowicki: All pull requests linked via external trackers have merged:

Bugzilla bug 2042036 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2042036: remove options for existing resources which are currently not supported

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants