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

asset/manifests: update status of Infrastructure #1155

Merged
merged 2 commits into from
Feb 1, 2019
Merged

asset/manifests: update status of Infrastructure #1155

merged 2 commits into from
Feb 1, 2019

Conversation

abhinavdahiya
Copy link
Contributor

This updates the Infrastructure.config.openshift.io object to catch up with 1.

Machine Config Operator uses the APIServerURL to point the kubelet on machines to apiserver, and EtcdDiscoveryDomain to setup the etcd
members to bootstrap the etcd cluster using dns discovery 2.

This is required as Machine Config Operator currently uses the cluster-config-v1 to creates these values and as cluster-config-v1 is getting deprecated 3, Machine Config Operator will move to
using the status of Infrastructure.config.openshift.io for sourcing that information.

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 30, 2019
@wking
Copy link
Member

wking commented Jan 30, 2019

/lgtm
/retest

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

wking commented Jan 31, 2019

e2e-aws:

Failing tests:

[Feature:Prometheus][Conformance] Prometheus when installed on the cluster should report telemetry if a cloud.openshift.com token is present [Suite:openshift/conformance/parallel/minimal]

But the telemetry folks just fixed a config issue, so...

/retest

@abhinavdahiya abhinavdahiya mentioned this pull request Jan 31, 2019
Merged
@openshift-bot
Copy link
Contributor

/retest

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

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

@wking
Copy link
Member

wking commented Jan 31, 2019

e2e-aws:

Failing tests:

[sig-apps] StatefulSet [k8s.io] Basic StatefulSet functionality [StatefulSetBasic] should adopt matching orphans and release non-matching pods [Suite:openshift/conformance/parallel] [Suite:k8s]
[sig-apps] StatefulSet [k8s.io] Basic StatefulSet functionality [StatefulSetBasic] should provide basic identity [Suite:openshift/conformance/parallel] [Suite:k8s]

both with panics :p. Just in case they're also flakes...

/retest

@wking
Copy link
Member

wking commented Jan 31, 2019

The should provide basic identity panic has shown up in around 1/6 of recent update-payload promotion checks, so flakiness confirmed. should adopt matching orphans... seems more stable. And I hear these will be fixed once we get a RHCOS with a 1.12 kubelet, but I'm not sure how close we are to that.

@wking wking mentioned this pull request Jan 31, 2019
Merged
@wking
Copy link
Member

wking commented Jan 31, 2019

e2e-aws:

Flaky tests:

[sig-network] Services should serve multiport endpoints from pods  [Conformance] [Suite:openshift/conformance/parallel/minimal] [Suite:k8s]

Failing tests:

[sig-apps] StatefulSet [k8s.io] Basic StatefulSet functionality [StatefulSetBasic] should provide basic identity [Suite:openshift/conformance/parallel] [Suite:k8s]

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@wking
Copy link
Member

wking commented Jan 31, 2019

Needs a rebase.

@openshift-bot
Copy link
Contributor

/retest

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

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 31, 2019
@abhinavdahiya
Copy link
Contributor Author

rebase around #1132

@abhinavdahiya
Copy link
Contributor Author

ping @wking

@abhinavdahiya
asset/manifests: update status of Infrastructure
218aa9b 
This updates the `Infrastructure.config.openshift.io` object to catch up with [1].

Machine Config Operator uses the `APIServerURL` to point the kubelet on machines to apiserver, and `EtcdDiscoveryDomain` to setup the etcd
members to bootstrap the etcd cluster using dns discovery [2].

This is required as Machine Config Operator currently uses the `cluster-config-v1` to creates these values and as `cluster-config-v1` is getting deprecated [3], Machine Config Operator will move to
using the status of `Infrastructure.config.openshift.io` for sourcing that information.

[1]: openshift/api#189
[2]: https://github.com/etcd-io/etcd/blob/583763261f1c843e07c1bf7fea5fb4cfb684fe87/Documentation/op-guide/clustering.md#dns-discovery
[3]: openshift#680
@abhinavdahiya
vendor: update github.com/openshift/api/config/v1
49719a1
@wking
Copy link
Member

wking commented Jan 31, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, 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:
  • OWNERS [abhinavdahiya,wking]

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

@abhinavdahiya abhinavdahiya mentioned this pull request Jan 31, 2019
Merged
6 tasks
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 11efd8e into openshift:master Feb 1, 2019
abhinavdahiya added a commit to abhinavdahiya/machine-config-operator that referenced this pull request Feb 12, 2019
@abhinavdahiya
apis: replace ClusterName, BaseDomain with EtcdDiscoveryDomain
88d3e94 
Since [1], the `Infrastructure.config.openshift.io` global config object contains the kube apiserver's url
and the domain that needs to be used by etcd to do bootstrapping using dns discovery.

The installer creates and sets the appropiate value since [2].

Therefore there is no longer a requirement for cluster_name and base_domain to create these values in template controller.

[1]: openshift/api#189
[2]: openshift/installer#1155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronlevy aaronlevy Awaiting requested review from aaronlevy

@steveej steveej Awaiting requested review from steveej

Assignees

@wking wking

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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@abhinavdahiya @wking @openshift-bot @openshift-ci-robot @openshift-merge-robot