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.

Sign up for GitHub

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

Jump to bottom

Add InfrastructureName. #266

Merged
merged 2 commits into from
Mar 29, 2019
Merged

Add InfrastructureName. #266

merged 2 commits into from
Mar 29, 2019

Conversation

dgoodwin
Copy link
Contributor

Picking up where @fabianofranz had to leave off in #231.

Adding InfrastructureName will give components the information they need to tag cloud resources consistently. (kubernetes.io/cluster/[infraname] = owned)

The controversial arbitrary AWS tagging is removed based on the comments in that PR.

I was going to move this back to Spec because it does not feel like Status to me, but I saw PlatformType is already Status and thus I think this ship has already sailed.

CC @abhinavdahiya @wking @derekwaynecarr @smarterclayton

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 26, 2019
@abhinavdahiya
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 26, 2019
@wking
Copy link
Member

wking commented Mar 26, 2019

/lgtm

@dgoodwin
Copy link
Contributor Author

@sdodson can you help us get this merged?

soltysh
soltysh approved these changes Mar 29, 2019
View reviewed changes
Copy link

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, dgoodwin, soltysh, 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 Mar 29, 2019
@openshift-merge-robot openshift-merge-robot merged commit e8532c5 into openshift:master Mar 29, 2019
wking added a commit to wking/openshift-installer that referenced this pull request Apr 1, 2019
@wking
vendor: Bump openshift/api to pick up InfrastructureName
3208111 
Catching up with openshift/api@c9b4e5b2d7 (Add InfrastructureName,
2019-03-01, openshift/api#266).

Generated with:

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

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

The client-go bump avoids:

  vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/features.go:22:25: undefined: "github.com/openshift/installer/vendor/github.com/openshift/api/config/v1".Features

by pulling in openshift/client-go@035cd9bc (bump API and regenerate
for FeatureGate API, 2019-03-13, openshift/client-go#97) to match
openshift/api@ee961910 (features: rename features to FeatureGate,
2019-03-05, openshift/api#247), which was also pulled in by the API
bump.
wking added a commit to wking/openshift-installer that referenced this pull request Apr 1, 2019
@wking
pkg/asset/manifests/infrastructure: Set InfrastructureName
2bd24cd 
So in-cluster components can pull the value and set their
kubernetes.io/cluster/{infraName} tags appropriately.

Also adjust to the AWSPlatform -> AWSPlatformType changes and similar
from openshift/api@c9b4e5b2d7 (Add InfrastructureName, 2019-03-01,
openshift/api#266).
@wking wking mentioned this pull request Apr 1, 2019
Merged
ingvagabund added a commit to ingvagabund/machine-api-operator that referenced this pull request Apr 3, 2019
@ingvagabund
Rename configv1 provider platform constants
0905307 
All AWSPlatform, AzurePlatform, etc. got renamed to AWSPlatformType, AzurePlatformType, etc.
See openshift/api#266 for more context.
ingvagabund added a commit to ingvagabund/machine-api-operator that referenced this pull request Apr 3, 2019
@ingvagabund
Rename configv1 provider platform constants
04a6dac 
All AWSPlatform, AzurePlatform, etc. got renamed to AWSPlatformType, AzurePlatformType, etc.
See openshift/api#266 for more context.
ingvagabund added a commit to ingvagabund/machine-api-operator that referenced this pull request Apr 3, 2019
@ingvagabund
Rename configv1 provider platform constants
3ad219c 
All AWSPlatform, AzurePlatform, etc. got renamed to AWSPlatformType, AzurePlatformType, etc.
See openshift/api#266 for more context.
@wking wking mentioned this pull request Apr 22, 2019
Closed
wking added a commit to wking/cluster-image-registry-operator that referenced this pull request Apr 23, 2019
@wking
vendor: Bump openshift/api
1e4baa8 
Because I need InfrastructureName from openshift/api@c9b4e5b2d7 (Adds
platform-specific specs to Infrastructure in config.openshift.io,
2019-03-01, openshift/api#266) in order to drop the cluster-config-v1
stuff here.

Generated with:

  $ sed -i s/ea5d05408a95a765d44b5a4b31561b530f0b1f4c/ea5d05408a95a765d44b5a4b31561b530f0b1f4c/ Gopkg.toml  # openshift/api
  $ sed -i s/31ef839c86359881d2c312c4f0131febc6662400/0255926f53935175fe90b8e7672c4c06c17d79e6/ Gopkg.toml  # openshift/client-go, to avoid "undefined Features" errors after the API bump
  $ emacs Gopkg.toml  # remove openshift/installer, now that we're using the Infrastructure config
  $ dep ensure

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 added a commit to wking/cluster-image-registry-operator that referenced this pull request Apr 23, 2019
@wking
pkg: Migrate from cluster-config-v1 to Infrastructure.config.openshif…
cc3b2c9 
…t.io

Mostly, this is replacing cluster-config-v1 (which is deprecated [1])
with the Infrastructure config object.  As part of that, I'm dropping
UserTags support (see discussion in [2,3]).  I'm also replacing the
openshiftClusterID tag with a "kubernetes.io/cluster/{infraName}:
owned" tag [4,5].

[1]: openshift/installer#680
[2]: openshift/api#231 (comment)
[3]: openshift/api#266
[4]: openshift/installer#1280
[5]: https://bugzilla.redhat.com/show_bug.cgi?id=1685089#c11
@wking wking mentioned this pull request Apr 23, 2019
Closed
wking added a commit to wking/cluster-image-registry-operator that referenced this pull request Apr 23, 2019
@wking
pkg: Migrate from cluster-config-v1 to Infrastructure.config.openshif…
7244438 
…t.io

Mostly, this is replacing cluster-config-v1 (which is deprecated [1])
with the Infrastructure config object.  As part of that, I'm dropping
UserTags support (see discussion in [2,3]).  I'm also replacing the
openshiftClusterID tag with a "kubernetes.io/cluster/{infraName}:
owned" tag [4,5].

UnversionedRESTClientFor (vs. RESTClientFor) avoids errors like:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1304/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-fc9948cc-b767n_cluster-image-registry-operator.log.gz | gunzip
  I0423 05:43:03.599910       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g38fc8f9-dirty
  ...
  I0423 05:43:04.942285       1 bootstrap.go:38] generating registry custom resource
  E0423 05:43:04.942775       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: GroupVersion is required when initializing a RESTClient, requeuing

InClusterConfig does not set GroupVersion [6] and RESTClientFor
requires it [7] while UnversionedRESTClientFor can fill it in [8].

[1]: openshift/installer#680
[2]: openshift/api#231 (comment)
[3]: openshift/api#266
[4]: openshift/installer#1280
[5]: https://bugzilla.redhat.com/show_bug.cgi?id=1685089#c11
[6]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L422-L428
[7]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L269-L270
[8]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L331-L333
wking added a commit to wking/cluster-image-registry-operator that referenced this pull request Apr 23, 2019
@wking
pkg: Migrate from cluster-config-v1 to Infrastructure.config.openshif…
843c843 
…t.io

Mostly, this is replacing cluster-config-v1 (which is deprecated [1])
with the Infrastructure config object.  As part of that, I'm dropping
UserTags support (see discussion in [2,3]).  I'm also replacing the
openshiftClusterID tag with a "kubernetes.io/cluster/{infraName}:
owned" tag [4,5].

The trip through clientcorev1.NewForConfig is because calling
RESTClientFor on the InClusterConfig raises:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1304/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-fc9948cc-b767n_cluster-image-registry-operator.log.gz | gunzip
  I0423 05:43:03.599910       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g38fc8f9-dirty
  ...
  I0423 05:43:04.942285       1 bootstrap.go:38] generating registry custom resource
  E0423 05:43:04.942775       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: GroupVersion is required when initializing a RESTClient, requeuing

InClusterConfig does not set GroupVersion [6] and RESTClientFor
requires it [7] while UnversionedRESTClientFor can fill it in [8].
But UnversionedRESTClientFor leaves NegotiatedSerializer unset:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1305/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-66d8b56784-5wt9v_cluster-image-registry-operator.log.gz | gunzip
  I0423 09:21:42.998300       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g64d5f62-dirty
  ...
  I0423 09:21:44.345677       1 bootstrap.go:38] generating registry custom resource
  E0423 09:21:44.346403       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: NegotiatedSerializer is required when initializing a RESTClient, requeuing

So I'm washing through clientcorev1.NewForConfig to get defaults for
both [9].

[1]: openshift/installer#680
[2]: openshift/api#231 (comment)
[3]: openshift/api#266
[4]: openshift/installer#1280
[5]: https://bugzilla.redhat.com/show_bug.cgi?id=1685089#c11
[6]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L422-L428
[7]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L269-L270
[8]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L331-L333
[9]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/kubernetes/typed/core/v1/core_client.go#L144-L155
wking added a commit to wking/cluster-image-registry-operator that referenced this pull request Apr 23, 2019
@wking
pkg: Migrate from cluster-config-v1 to Infrastructure.config.openshif…
4b2bbe5 
…t.io

Mostly, this is replacing cluster-config-v1 (which is deprecated [1])
with the Infrastructure config object.  As part of that, I'm dropping
UserTags support (see discussion in [2,3]).  I'm also replacing the
openshiftClusterID tag with a "kubernetes.io/cluster/{infraName}:
owned" tag [4,5].

The trip through clientcorev1.NewForConfig is because calling
RESTClientFor on the InClusterConfig raises:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1304/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-fc9948cc-b767n_cluster-image-registry-operator.log.gz | gunzip
  I0423 05:43:03.599910       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g38fc8f9-dirty
  ...
  I0423 05:43:04.942285       1 bootstrap.go:38] generating registry custom resource
  E0423 05:43:04.942775       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: GroupVersion is required when initializing a RESTClient, requeuing

InClusterConfig does not set GroupVersion [6] and RESTClientFor
requires it [7] while UnversionedRESTClientFor can fill it in [8].
But UnversionedRESTClientFor leaves NegotiatedSerializer unset:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1305/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-66d8b56784-5wt9v_cluster-image-registry-operator.log.gz | gunzip
  I0423 09:21:42.998300       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g64d5f62-dirty
  ...
  I0423 09:21:44.345677       1 bootstrap.go:38] generating registry custom resource
  E0423 09:21:44.346403       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: NegotiatedSerializer is required when initializing a RESTClient, requeuing

So I'm washing through clientcorev1.NewForConfig to get defaults for
both [9].

The RBAC change avoids:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1306/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-7f59899bf6-8c8x8_cluster-image-registry-operator.log.gz | gunzip | head -n7
  I0423 10:44:51.121540       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-gfa5b59b-dirty
  ...
  I0423 10:44:52.461691       1 bootstrap.go:38] generating registry custom resource
  E0423 10:44:52.464035       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: infrastructures "cluster" is forbidden: User "system:serviceaccount:openshift-image-registry:cluster-image-registry-operator" cannot get resource "infrastructures" in API group "" at the cluster scope, requeuing

[1]: openshift/installer#680
[2]: openshift/api#231 (comment)
[3]: openshift/api#266
[4]: openshift/installer#1280
[5]: https://bugzilla.redhat.com/show_bug.cgi?id=1685089#c11
[6]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L422-L428
[7]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L269-L270
[8]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L331-L333
[9]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/kubernetes/typed/core/v1/core_client.go#L144-L155
wking added a commit to wking/cluster-image-registry-operator that referenced this pull request Apr 23, 2019
@wking
pkg: Migrate from cluster-config-v1 to Infrastructure.config.openshif…
bb90d24 
…t.io

Mostly, this is replacing cluster-config-v1 (which is deprecated [1])
with the Infrastructure config object.  As part of that, I'm dropping
UserTags support (see discussion in [2,3]).  I'm also replacing the
openshiftClusterID tag with a "kubernetes.io/cluster/{infraName}:
owned" tag [4,5].

The trip through clientcorev1.NewForConfig is because calling
RESTClientFor on the InClusterConfig raises:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1304/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-fc9948cc-b767n_cluster-image-registry-operator.log.gz | gunzip
  I0423 05:43:03.599910       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g38fc8f9-dirty
  ...
  I0423 05:43:04.942285       1 bootstrap.go:38] generating registry custom resource
  E0423 05:43:04.942775       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: GroupVersion is required when initializing a RESTClient, requeuing

InClusterConfig does not set GroupVersion [6] and RESTClientFor
requires it [7] while UnversionedRESTClientFor can fill it in [8].
But UnversionedRESTClientFor leaves NegotiatedSerializer unset:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1305/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-66d8b56784-5wt9v_cluster-image-registry-operator.log.gz | gunzip
  I0423 09:21:42.998300       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g64d5f62-dirty
  ...
  I0423 09:21:44.345677       1 bootstrap.go:38] generating registry custom resource
  E0423 09:21:44.346403       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: NegotiatedSerializer is required when initializing a RESTClient, requeuing

So I'm washing through clientcorev1.NewForConfig to get defaults for
both [9].

The RBAC change avoids:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1306/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-7f59899bf6-8c8x8_cluster-image-registry-operator.log.gz | gunzip | head -n7
  I0423 10:44:51.121540       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-gfa5b59b-dirty
  ...
  I0423 10:44:52.461691       1 bootstrap.go:38] generating registry custom resource
  E0423 10:44:52.464035       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: infrastructures "cluster" is forbidden: User "system:serviceaccount:openshift-image-registry:cluster-image-registry-operator" cannot get resource "infrastructures" in API group "" at the cluster scope, requeuing

[1]: openshift/installer#680
[2]: openshift/api#231 (comment)
[3]: openshift/api#266
[4]: openshift/installer#1280
[5]: https://bugzilla.redhat.com/show_bug.cgi?id=1685089#c11
[6]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L422-L428
[7]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L269-L270
[8]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L331-L333
[9]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/kubernetes/typed/core/v1/core_client.go#L144-L155
@wking wking mentioned this pull request Apr 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soltysh soltysh soltysh approved these changes

@abhinavdahiya abhinavdahiya Awaiting requested review from abhinavdahiya

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

Assignees

@wking wking

@soltysh soltysh

@abhinavdahiya abhinavdahiya

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@dgoodwin @abhinavdahiya @wking @openshift-ci-robot @soltysh @openshift-merge-robot @fabianofranz