Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/asset/manifests/infrastructure: Set status.platformStatus.aws.region #1725

Closed
wants to merge 2 commits into from

Conversation

wking
Copy link
Member

@wking wking commented May 7, 2019

Builds on #1718; I'll rebase after that lands.

Using openshift/api@dedfb47b1f (openshift/api#300) so the registry can use it as a default for their config resource.

This will allow the registry to move off the deprecated cluster-config-v1 (#680) and make it easier for them to move from openshiftClusterID tagging to kubernetes.io/cluster/{infraName} tagging (openshift/cluster-image-registry-operator#267), which is part of addressing rhbz#1685089.

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 7, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

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 /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 May 7, 2019
@wking wking force-pushed the api-infrastructure-aws-region branch 2 times, most recently from f91803f to c528318 Compare May 7, 2019 19:25
@wking wking force-pushed the api-infrastructure-aws-region branch from c528318 to ecaf8bf Compare May 7, 2019 20:45
wking added 2 commits May 8, 2019 09:08
Pulling in openshift/api@dedfb47b1f (config/v1/types_infrastructure:
Add AWS Region, 2019-04-26, openshift/api#300).

Generated with:

  $ dep ensure -update github.com/openshift/api

using:

  $ dep version
  dep:
   version     : v0.5.1
   build date  : 2019-03-20
   git hash    : faa61893
   go version  : go1.10.3
   go compiler : gc
   platform    : linux/amd64
   features    : ImportDuringSolve=false
@wking wking force-pushed the api-infrastructure-aws-region branch from ecaf8bf to 8b3b375 Compare May 8, 2019 16:10
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 8, 2019
@wking
Copy link
Member Author

wking commented May 8, 2019

Rebased onto master with ecaf8bf -> 8b3b375 now that #1718 has landed. Also fixed the nil-dereference bug.

@openshift-ci-robot
Copy link
Contributor

@wking: PR needs rebase.

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 openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2019
ironcladlou added a commit to ironcladlou/cluster-ingress-operator that referenced this pull request May 14, 2019
Access to ec2 metadata will soon be restricted
(openshift/origin#22826). Eliminate the ec2 metadata
dependency by discovering AWS region information from cluster config. This
commit uses the deprecated install config for metatadata; once
openshift/installer#1725 merges, supported cluster
config will provide the region information and the code can be refactored.
ironcladlou added a commit to ironcladlou/cluster-ingress-operator that referenced this pull request May 14, 2019
Access to ec2 metadata will soon be restricted
(openshift/origin#22826). Eliminate the ec2 metadata
dependency by discovering AWS region information from cluster config. This
commit uses the deprecated install config for metatadata; once
openshift/installer#1725 merges, supported cluster
config will provide the region information and the code can be refactored.
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/cluster-ingress-operator that referenced this pull request May 15, 2019
Access to ec2 metadata will soon be restricted
(openshift/origin#22826). Eliminate the ec2 metadata
dependency by discovering AWS region information from cluster config. This
commit uses the deprecated install config for metatadata; once
openshift/installer#1725 merges, supported cluster
config will provide the region information and the code can be refactored.
@openshift-ci-robot
Copy link
Contributor

@wking: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-upgrade 8b3b375 link /test e2e-aws-upgrade
ci/prow/e2e-openstack 8b3b375 link /test e2e-openstack
ci/prow/images 8b3b375 link /test images
ci/prow/e2e-aws 8b3b375 link /test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@abhinavdahiya
Copy link
Contributor

@wking We would need a plan to set this up in existing clusters to make upgrades possible.

Define a job in some operartor that ends up in release image that sets this field from cluster-config-v1.
before we merge this change

case vsphere.Name:
config.Status.PlatformStatus.Type = configv1.VSpherePlatformType
case azure.Name:
config.Status.PlatformStatus.Type = configv1.AzurePlatformType
Copy link

@dmage dmage May 23, 2019

Choose a reason for hiding this comment

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

We also need LocationID for Azure, so that we know in which region we should create Storage Account.

@coreydaley
Copy link
Member

@wking bump

@sgreene570
Copy link

friendly ping on this 😃

@celebdor
Copy link
Contributor

@abhinavdahiya @wking any idea when this will get in? @cybertron needs to add info to the Baremetal PlatformStatus and I'm wondering if this needs to be getting merged first.

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. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants