Skip to content

Conversation

@ironcladlou
Copy link
Contributor

@ironcladlou ironcladlou commented Oct 17, 2019

Render ingress operator manifests during bootstrap. This enables users to bring
their own default ingresscontroller resource by using openshift-install create manifests and placing ingresscontroller YAML into the manifests/ directory.

At the time of this commit, the rendered output on the bootstrap node will be
the ingressoperator CRD[1] and namespace[2].

This is the first part of the "User Defined Default Ingress Controller"
enhancement proposal[3].

[1] https://github.com/openshift/cluster-ingress-operator/blob/master/manifests/00-custom-resource-definition-internal.yaml
[2] https://github.com/openshift/cluster-ingress-operator/blob/master/manifests/00-namespace.yaml
[3] https://github.com/openshift/enhancements/blob/master/enhancements/user-defined-default-ingress-controller.md

TODO:

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 17, 2019
@ironcladlou ironcladlou force-pushed the default-ingress-controller branch from 4e271fe to a9a2a1c Compare October 18, 2019 00:06
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 18, 2019
@ironcladlou ironcladlou changed the title WIP: Enable bring-your-own ingresscontrollers Enable user defined default ingresscontroller Oct 18, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2019
@ironcladlou
Copy link
Contributor Author

Depends on openshift/cluster-ingress-operator#309.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 18, 2019
@ironcladlou
Copy link
Contributor Author

@abhinavdahiya for the first revision I've pared it down to the absolute minimum required. The next followup can be integration with the publishing strategy from #2521.

@ironcladlou
Copy link
Contributor Author

@abhinavdahiya openshift/cluster-ingress-operator#309 merged, this is ready for review.

/retest
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 18, 2019
@abhinavdahiya
Copy link
Contributor

Would you mind including in the commit of the PR body, the output of the render command on the bootstrap-host.

@abhinavdahiya
Copy link
Contributor

/test e2e-gcp

/test e2e-azure

/test e2e-aws-upi

Render ingress operator manifests during bootstrap. This enables users to bring
their own default ingresscontroller resource by using `openshift-install create
manifests` and placing ingresscontroller YAML into the `manifests/` directory.

At the time of this commit, the rendered output on the bootstrap node will be
the ingressoperator CRD[1] and namespace[2].

This is the first part of the "User Defined Default Ingress Controller"
enhancement proposal[3].

[1] https://github.com/openshift/cluster-ingress-operator/blob/master/manifests/00-custom-resource-definition-internal.yaml
[2] https://github.com/openshift/cluster-ingress-operator/blob/master/manifests/00-namespace.yaml
[3] https://github.com/openshift/enhancements/blob/master/enhancements/user-defined-default-ingress-controller.md
@ironcladlou ironcladlou force-pushed the default-ingress-controller branch from a9a2a1c to 13adb6b Compare October 18, 2019 14:59
@ironcladlou
Copy link
Contributor Author

Would you mind including in the commit of the PR body, the output of the render command on the bootstrap-host.

I added some links instead, because the contents are huge and should be opaque to the installer anyway.

@abhinavdahiya
Copy link
Contributor

/test e2e-gcp

/test e2e-azure

/test e2e-aws-upi

@abhinavdahiya
Copy link
Contributor

Would you mind including in the commit of the PR body, the output of the render command on the bootstrap-host.

I added some links instead, because the contents are huge and should be opaque to the installer anyway.

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 18, 2019
@ironcladlou
Copy link
Contributor Author

@abhinavdahiya can I get a lgtm on this?

@abhinavdahiya
Copy link
Contributor

@ironcladlou waiting for some optional platform specific PRs to go green enough and i'll drop an lgtm then.

@abhinavdahiya
Copy link
Contributor

/test e2e-metal

@abhinavdahiya
Copy link
Contributor

/test e2e-aws-upi

@abhinavdahiya
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, ironcladlou

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

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

@openshift-merge-robot openshift-merge-robot merged commit adb4218 into openshift:master Oct 19, 2019
@openshift-ci-robot
Copy link
Contributor

@ironcladlou: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-upi 13adb6b link /test e2e-aws-upi

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.

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.

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. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants