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

Remove cluster-api cluster object dependency #1449

Merged
merged 2 commits into from
Mar 25, 2019
Merged

Remove cluster-api cluster object dependency #1449

merged 2 commits into from
Mar 25, 2019

Conversation

enxebre
Copy link
Member

@enxebre enxebre commented Mar 21, 2019 •
edited
Loading

Fix https://jira.coreos.com/browse/CLOUD-357
The cluster object was necessary for the machine controllers to function. This dep is being removed and there's no reason for this object to exist at the moment
Needs:
openshift/cluster-kube-apiserver-operator#351
openshift/cluster-kube-controller-manager-operator#196
This won't go green until the changes in these two PRs make it to the payload

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 21, 2019
@enxebre enxebre mentioned this pull request Mar 21, 2019
Merged
@abhinavdahiya abhinavdahiya self-assigned this Mar 21, 2019
@abhinavdahiya
Copy link
Contributor

/retest

@enxebre enxebre mentioned this pull request Mar 22, 2019
Merged
@enxebre enxebre force-pushed the remove-cluster-object-dep branch from 50550a9 to 88901fa Compare March 22, 2019 09:17
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 22, 2019
@enxebre
Remove cluster-api cluster api file dependency for bootstrapping
2177408 
The cluster object was necessary for the machine controllers to function. This dep is being removed and there's no reason for this object to exist at the moment
@enxebre
Copy link
Member Author

enxebre commented Mar 25, 2019

second PR went through here https://origin-release.svc.ci.openshift.org/releasestream/4.0.0-0.alpha/release/4.0.0-0.alpha-2019-03-22-231511
/retest

@enxebre
Copy link
Member Author

enxebre commented Mar 25, 2019

ping @abhinavdahiya @wking

wking
wking approved these changes Mar 25, 2019
View reviewed changes
Copy link
Member

@wking wking 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-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 25, 2019
@abhinavdahiya
Copy link
Contributor

it looks like you might have to re-roll https://github.com/openshift/installer/blob/88901fa7fbeec8e42a918cc69d989af0b52f7c64/pkg/asset/manifests/network.go#L128 to remove dep on cluster1a1

@abhinavdahiya
Copy link
Contributor

/lgtm

it looks like you might have to re-roll

installer/pkg/asset/manifests/network.go

Line 128 in 88901fa
func (no *Networking) ClusterNetwork() (*clusterv1a1.ClusterNetworkingConfig, error) {
to remove dep on cluster1a1

^^ this function seems to be not used at all. maybe just drop this. it was being consumed 88901fa#diff-a25ec653fe393c89c4ccac5a1edfe38fL42

/lgtm cancel

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2019
@enxebre enxebre force-pushed the remove-cluster-object-dep branch from 88901fa to c734361 Compare March 25, 2019 14:08
@enxebre
Copy link
Member Author

enxebre commented Mar 25, 2019

back to green @abhinavdahiya

@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 25, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, enxebre, 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

@openshift-merge-robot openshift-merge-robot merged commit 48dbde1 into openshift:master Mar 25, 2019
@abhinavdahiya abhinavdahiya mentioned this pull request Mar 26, 2019
Merged
wking added a commit to wking/openshift-installer that referenced this pull request Apr 2, 2019
@wking
docs/design/resource_dep: Rebuild without ClusterK8sIO, etc.
71bf5a6 
Catching up with c734361 (Remove cluster-api object as this is not
needed anymore, 2019-03-22, openshift#1449), 1408d8a (*: use
kube-etcd-cert-signer release image, 2019-03-27, openshift#1477), and possibly
others.  Generated with:

  $ openshift-install graph | dot -Tsvg >docs/design/resource_dep.svg

using:

  $ dot -V
  dot - graphviz version 2.30.1 (20170916.1124)
@wking wking mentioned this pull request Apr 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wking wking wking approved these changes

Assignees

@wking wking

@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.
5 participants
@enxebre @abhinavdahiya @openshift-ci-robot @wking @openshift-merge-robot