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.

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

Creating initial tsb role to consume and apply templates provided for… #5226

Merged

Conversation

@ewolinetz
Copy link
Contributor Author

Changes dependent on openshift/origin#15993

@ewolinetz ewolinetz changed the title [WIP] Creating initial tsb role to consume and apply tepmlates provided for… [WIP] Creating initial tsb role to consume and apply templates provided for… Aug 31, 2017
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 8, 2017
@ewolinetz
Copy link
Contributor Author

@deads2k is this the correct way to handle the TSB config? I'm seeing this when I try to kubectl apply it

fatal: [m01.example.com]: FAILED! => {
    "changed": true, 
    "cmd": [
        "kubectl", 
        "apply", 
        "-f", 
        "/tmp/tsb-ansible-lS4vzn/apiserver-config.yaml"
    ], 
    "delta": "0:00:00.219908", 
    "end": "2017-09-08 16:51:36.256954", 
    "failed": true, 
    "invocation": {
        "module_args": {
            "_raw_params": "kubectl apply -f /tmp/tsb-ansible-lS4vzn/apiserver-config.yaml", 
            "_uses_shell": false, 
            "chdir": null, 
            "creates": null, 
            "executable": null, 
            "removes": null, 
            "warn": true
        }
    }, 
    "rc": 1, 
    "start": "2017-09-08 16:51:36.037046", 
    "stderr": "error: unable to recognize \"/tmp/tsb-ansible-lS4vzn/apiserver-config.yaml\": no matches for config.templateservicebroker.openshift.io/, Kind=TemplateServiceBrokerConfig", 
    "stderr_lines": [
        "error: unable to recognize \"/tmp/tsb-ansible-lS4vzn/apiserver-config.yaml\": no matches for config.templateservicebroker.openshift.io/, Kind=TemplateServiceBrokerConfig"
    ], 
    "stdout": "", 
    "stdout_lines": []
}

@deads2k
Copy link
Contributor

deads2k commented Sep 8, 2017

I'm afraid I don't speak ansible very well. You're doing oc process -f install/templateservicebroker/apiserver-template.yaml | oc apply -f - or equivalent?

@ewolinetz
Copy link
Contributor Author

@deads2k apologies... I'm doing kubectl apply -f /tmp/tsb-ansible-lS4vzn/apiserver-config.yaml should I do oc process -f .... | oc apply -f - ? My server returned saying that it doesn't recognize the object kind...

no matches for config.templateservicebroker.openshift.io/, Kind=TemplateServiceBrokerConfig

@deads2k
Copy link
Contributor

deads2k commented Sep 8, 2017

I'm doing kubectl apply -f /tmp/tsb-ansible-lS4vzn/apiserver-config.yaml

I don't recognize that file. Is it a template file or a processed template (list)? If it is https://github.com/openshift/origin/blob/master/install/templateservicebroker/apiserver-config.yaml , then that's the config file for the server which you will pass to the template as a parameter. You don't apply that one.

@ewolinetz
Copy link
Contributor Author

@deads2k ack, yes... I'm forgetting that there's that layer of abstraction. It is that config file... so when I create the server template I pass this as a parameter, got it! Much appreciated

@ewolinetz ewolinetz changed the title [WIP] Creating initial tsb role to consume and apply templates provided for… Creating initial tsb role to consume and apply templates provided for… Sep 8, 2017
@ewolinetz
Copy link
Contributor Author

@sdodson @deads2k any qualms with separating this work from the work required for #5056 ?

@sdodson
Copy link
Member

sdodson commented Sep 9, 2017

@sdodson @deads2k any qualms with separating this work from the work required for #5056 ?

No that's fine with me. #5056 would be addressed first?

@ewolinetz
Copy link
Contributor Author

They'd both need to be delivered for 3.7 but it might make sense to get #5056 merged in first

@@ -0,0 +1,25 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

What do we do when any of these commands fail? How do we ensure that these objects are removed? Just something to think about.

@ewolinetz
Copy link
Contributor Author

aos-ci-test

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 36829d0 (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for 36829d0 (logs)

@ewolinetz
Copy link
Contributor Author

/retest

@ewolinetz
Copy link
Contributor Author

@deads2k with the latest tested locally I see the TSB pods start up and run after an installation

@deads2k
Copy link
Contributor

deads2k commented Sep 19, 2017

@sdodson @kwoodson I haven't seen much review action on this. I don't know enough to meaningfully review it, but if it can get reviewed in time to prove the pattern you may be able to use it for something like service catalog this release too.


- set_fact:
openshift_metrics_storage_kind: "{{ openshift_hosted_metrics_storage_kind }}"
when: openshift_hosted_metrics_storage_kind is defined
Copy link
Member

Choose a reason for hiding this comment

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

This file seems unrelated to the topic of the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh, i committed while on the wrong branch locally... but i thought i reverted it. Will remove

with_items:
- "{{ __tsb_template_file }}"
- "{{ __tsb_config_file }}"
- "{{ __tsb_rbac_file }}"
Copy link
Member

Choose a reason for hiding this comment

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

Has this stabilized enough that we should go ahead and add this content to openshift-ansible in the location we agreed upon rather than fetching it from origin github?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the alternative to pulling the content from origin was going to be a locally sync'd place in ansible. Is that configured yet? We should be able to just change the source location in vars/

Copy link
Member

@sdodson sdodson Sep 20, 2017

Choose a reason for hiding this comment

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

Yeah, we'll setup CI tooling for that but lets go ahead and import a copy of current content as part of this PR and use that rather than fetching from github?


- name: Delete template file
command: >
kubectl delete -f {{ mktemp.stdout }}/{{ __tsb_template_file }}
Copy link
Member

Choose a reason for hiding this comment

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

kubectl versus oc in the next task, is that intentional?

Copy link
Member

Choose a reason for hiding this comment

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

@deads2k delete on a template will delete all the resources defined in the template?

Copy link
Contributor

Choose a reason for hiding this comment

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

@deads2k delete on a template will delete all the resources defined in the template?

You need to process the template, then pipe that to delete. You probably want to delete the entire namespace (after doing this) just in case.

Copy link
Member

Choose a reason for hiding this comment

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

I was going to suggest just nuking the namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to suggest just nuking the namespace.

both are useful. Some resources which are created are cluster scoped. ClusterRoles for instance.

@sdodson
Copy link
Member

sdodson commented Sep 19, 2017

Assuming the metrics related stuff is unrelated to this PR lets remove that then I'm LGTM.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2017
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2017
@ewolinetz
Copy link
Contributor Author

/retest

@ewolinetz
Copy link
Contributor Author

@sdodson @deads2k how does everything look?

Copy link
Member

@sdodson sdodson left a comment

Choose a reason for hiding this comment

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

LGTM

@sdodson
Copy link
Member

sdodson commented Sep 21, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 21, 2017
@openshift-merge-robot
Copy link
Contributor

/test all [submit-queue is verifying that this PR is safe to merge]

@sdodson
Copy link
Member

sdodson commented Sep 22, 2017

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue

@openshift-merge-robot openshift-merge-robot merged commit 2adb0eb into openshift:master Sep 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

7 participants