payload/render: update the bootstrap render to skip servicemonitor - #221
Conversation
the `manifests` directory on the bootstrap is used by the cluster-bootstrap to push to the cluster. `servicemonitor` for cvo was added by openshift#214 `servicemonitor` api is created by the cluster-monitoring-operator and therefore this causes the bootstrapping to get stuck until we get the monitoring operator running. This skips the `servicemonitor` in the bootstrap render as it is not required for the bootstrap cvo pod.
|
@abhinavdahiya: GitHub didn't allow me to request PR reviews from the following users: brancz. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
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
added
approved
|
/retest |
|
/approve |
|
Seems ok to me |
|
/retest |
|
We were under the assumption that because of the run level this would already be applied at a later point. I’ll lgtm this and put hold on it, feel free to remove the hold, if that assumption was incorrect. /lgtm |
openshift-ci-robot
added
the
do-not-merge/hold
|
@brancz: changing LGTM is restricted to collaborators DetailsIn response to this:
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. |
|
Bootstrap is special. Maybe we need an annotation that excludes things from bootstrap render? |
|
|
Don’t we parse after templating before we render? Or is bootstrap
sufficiently different?
Not a huge deal
On Jul 12, 2019, at 5:47 PM, Abhinav Dahiya <notifications@github.com> wrote:
Bootstrap is special. Maybe we need an annotation that excludes things from
bootstrap render?
@smarterclayton <https://github.com/smarterclayton>
the list of things right now is small (2) and if this get's bigger we can
think about that. and k8s annotations would mean parsing the file... *not
thrilled*
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#221?email_source=notifications&email_token=AAI37J52AKDTNLSIJKNAL43P7D3YRA5CNFSM4IBRD56KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ26WKA#issuecomment-511044392>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAI37J2MGZFJYZMXHCR5Q2LP7D3YRANCNFSM4IBRD56A>
.
|
|
/hold cancel the bootstrap render is reading, templating (text based) and copying,, it treats them as text file and not k8s manifests. So the annotation part would be a change from it being simple.. for now, I don't think the marshalling into k8s objects to make skipping on render is of great value. ping @smarterclayton |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/lgtm Sorry |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, brancz, crawford, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
7 participants
the
manifestsdirectory on the bootstrap is used by the cluster-bootstrap to push to the cluster.servicemonitorfor cvo was added by #214servicemonitorapi is created by the cluster-monitoring-operator and therefore this causes the bootstrapping to get stuck until we get the monitoring operator running.This skips the
servicemonitorin the bootstrap render as it is not required for the bootstrap cvo pod./cc @smarterclayton @brancz