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

Bug 1498954 - Broker in developer mode must support apb push #476

Merged
merged 1 commit into from Oct 6, 2017

Conversation

djzager
Copy link
Member

@djzager djzager commented Oct 5, 2017

Testing for this PR

Just used the apb tool to push an image to the broker and looked at how the image was stored by looking at the logs of the broker.

Before Change:

[2017-10-05T14:22:55.126Z] [DEBUG] handler::apbAddSpec
[2017-10-05T14:22:55.126Z] [DEBUG] Successfully decoded pushed spec:
[2017-10-05T14:22:55.126Z] [DEBUG] version: 1.0
name: hello-world-apb
description: deploys hello-world web application
bindable: False
async: optional
metadata:
  displayName: Hello World (APB)
  longDescription: A sample APB which deploys a containerized Hello World web application
  dependencies: ['docker.io/ansibleplaybookbundle/hello-world:latest']
  providerDisplayName: "Red Hat, Inc."
plans:
  - name: default
    description: A sample APB which deploys Hello World
    free: True
    metadata:
      displayName: Default
      longDescription: This plan deploys a Python web application displaying Hello World
      cost: $0.00
    parameters: []
 
[2017-10-05T14:22:55.127Z] [DEBUG] Unmarshalled into apb.Spec:
[2017-10-05T14:22:55.127Z] [DEBUG] {ID: Version:1.0 FQName:hello-world-apb Image: Tags:[] Bindable:false Description:deploys hello-world web application Metadata:map[displayName:Hello World (APB) longDescription:A sample APB which deploys a containerized Hello World web application dependencies:[docker.io/ansibleplaybookbundle/hello-world:latest] providerDisplayName:Red Hat, Inc.] Async:optional Plans:[{Name:default Description:A sample APB which deploys Hello World Metadata:map[displayName:Default longDescription:This plan deploys a Python web application displaying Hello World cost:$0.00] Free:true Bindable:false Parameters:[]}]}

After Change:

[2017-10-05T15:23:16.817Z] [DEBUG] handler::apbAddSpec
[2017-10-05T15:23:16.817Z] [DEBUG] Successfully decoded pushed spec:
[2017-10-05T15:23:16.817Z] [DEBUG] version: 1.0
name: hello-world-apb
description: deploys hello-world web application
bindable: False
async: optional
metadata:
  displayName: Hello World (APB)
  longDescription: A sample APB which deploys a containerized Hello World web application
  dependencies: ['docker.io/ansibleplaybookbundle/hello-world:latest']
  providerDisplayName: "Red Hat, Inc."
plans:
  - name: default
    description: A sample APB which deploys Hello World
    free: True
    metadata:
      displayName: Default
      longDescription: This plan deploys a Python web application displaying Hello World
      cost: $0.00
    parameters: []
 
[2017-10-05T15:23:16.817Z] [DEBUG] Unmarshalled into apb.Spec:
[2017-10-05T15:23:16.818Z] [DEBUG] {ID: Version:1.0 FQName:hello-world-apb Image:hello-world-apb Tags:[] Bindable:false Description:deploys hello-world web application Metadata:map[displayName:Hello World (APB) longDescription:A sample APB which deploys a containerized Hello World web application dependencies:[docker.io/ansibleplaybookbundle/hello-world:latest] providerDisplayName:Red Hat, Inc.] Async:optional Plans:[{Name:default Description:A sample APB which deploys Hello World Metadata:map[displayName:Default longDescription:This plan deploys a Python web application displaying Hello World cost:$0.00] Free:true Bindable:false Parameters:[]}]}

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 5, 2017
@djzager
Copy link
Member Author

djzager commented Oct 5, 2017

@karmab If you were able to test this change for your purposes, I would appreciate it.

dymurray
dymurray previously approved these changes Oct 5, 2017
Copy link
Member

@dymurray dymurray left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -564,6 +564,7 @@ func (h handler) apbAddSpec(w http.ResponseWriter, r *http.Request, params map[s
writeResponse(w, http.StatusBadRequest, broker.ErrorResponse{Description: "Invalid parameter yaml"})
return
}
spec.Image = spec.FQName
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels weird to put this in the handler. I would put it in AddSpec of broker.go.

Copy link
Member

Choose a reason for hiding this comment

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

+1 putting it in AddSpec makes more sense because that's what gets called when you push to the dev endpoint.

@karmab
Copy link
Contributor

karmab commented Oct 5, 2017

@djzager thanks for the change, do i have to build everything in order to test this one?

@karmab
Copy link
Contributor

karmab commented Oct 6, 2017

i tested the PR and didnt get the error

@dymurray dymurray dismissed their stale review October 6, 2017 12:15

I agree with Jesus' comment

@jmrodri jmrodri merged commit 206ceda into openshift:master Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants