[DEVEXP-457] Create app from source

[jcantrill]

@liggitt @jwforres Please do an initial review with regards to structure and what I'm pushed for filters, directives, specs, routes to see if I am moving in the direction you would expect

[jcantrill]

@jwforres This latest commit wires the 'create' action to the back-end. Still need to add some validations around validating labels, validating a user entering appname, warning a user if a generated appname is taken. Also needs some testing to confirm the resources are all wired correctly.

[jcantrill]

@jwforres Added validations and fixes to post resources to the server that will actually trigger a build. I think its ready to be evaluated by a wider audience. Also, I think we should decide if the way I implemented the routing functionality is what we really desire and if the UI makes sense for routing. The styling could also use another once over.
Maybe @pweil- should have a look to see its reasonable to him as well.
cc @bparees @csrwng @EmilyDirsh @sg00dwin

[jwforres]

See comments in below screenshots:

Also, not sure i like the "Specify route" label in combination with "Create a route for me". Seems like specify should be more like "Let me specify a route" or "I have a route already"

[liggitt]

Can we combine the "create for me" and "specify" cases, and just put a default value in the route text box?

Also, instead of a checkbox for secure/insecure, can we put a combo in front of the text field that lets you select "http://" or "https://"? I think that would clarify a few things: 1) what the option does to the route, 2) that you don't need the scheme in the route textbox

[liggitt]

Also, should we have the cert/key/ca be selected as a file browse instead of pasting in content?

[jwforres]

@jcantrill where do you get the auto-generated route from, is that something you have hard-coded?

Should check with @pweil- and @smarterclayton related to the skydns stuff @smarterclayton added. We probably need to send another config back to the web console in config.js for the subdomain that routes will be auto-generated at.

[pweil-]

/cc @ramr Ram, it looks like this could use a hook into your route allocator code merged in #1291 for auto-generating route names

[jwforres]

See comment in screenshot for style fix on scaling input. Also should we be letting them specify 0 for scaling, right now you have min set to 1, but I could see someone wanting to get everything set up with 0 scale to start with.

[jwforres]

"Automatically build new images when code changes" is a deceptive label, we aren't doing that level of magic for them. Need to be more clear about what exactly we are giving them (which are webhooks they can then use).

[jwforres]

Possibly instead of "Autodeploy when a new image is available " make it "Autodeploy when a new builder image is available" so it is clear what image we are talking about?

[jwforres]

Don't forget to update the help tip for scaling. Say something about it controlling the number of running instances of your built image.

[jwforres]

nothing tells me what the "Name" is actually being used for, I may want to know so that it helps me decide what to call it

[csrwng]

is there a reason to prefix these with 'osc' (I would think all directives are part of the openshift console)?

[jwforres]

this was the discussion we had the other day about prefixing our directives, its apparently a "best practice" to prefix them, so . It is debatable whether the file name really needs to have the prefix since in some of these files we are putting multiple directives.

[csrwng]

If we do decide to prefix the files, it'd make sense to eventually prefix all of them

[bparees]

sorry, jumping in late, but it looks like we're missing an option for "rebuild when builder image changes" or did we decide that's an expert mode configuration and we'll just default it to true here?

[bparees]

also we're not doing any validation on the source-url input field? (at least confirming it's a valid url?)

[jcantrill]

@bparees Correct there is no source url validation. @jwforres ??? In regards to rebuild when builder image changes is that "Autodeploy when a new builder image is available"?

[bparees]

@jcantrill no, and in fact i think you've got mixed wording there. will comment directly on that code.

but in any case deploy is different from build.

[bparees]

assuming this is ties to line 152, this line should not say "builder", it's just autodeploy when a new image is available.

what this should translate into is "when a new version of my application image is created, either because i manually pushed something, or a build pushed something, redeploy my app"

That's separate from what I think is still missing here which is "if my builder image (eg ruby-20-centos7) changed, rebuild my application". That rebuild might then trigger a redeploy, but that's a separate issue.

[jcantrill]

@jwforres @bparees @csrwng @sg00dwin Rebased to latest and updated to address previous comments(routing, key/value wigit for labels and env vars). It really needs some styling help especially in regards to the file upload and routing stuff

[jcantrill]

@jwforres @bparees @sg00dwin Updated PR to add imageChange trigger

[bparees]

imagechange bits look right.

[jwforres]

I feel like all these generate functions should really be abstracted out. Some kind of service that generates JSON for a type based on a set of inputs. The second we want to create/update individual components we are going to have to duplicate this.

Significant set of changes at this point, we can do that as a refactor later.

[jcantrill]

I would say that's a strong maybe. Update operations would allow us to
modify the existing json so I'm not sure it applies. Also, creating
individual resources I would think we might want to offer a user more
options then the simplified version of 'Generate Application'.

On 04/08/2015 03:48 PM, Jessica Forrester wrote:

In assets/app/scripts/services/applicationGenerator.js
#1221 (comment):

•        return this.name + ":" + this.tag;
  

•      }
  

•    };
  

•  }
  

•  var resources = {
  

•    imageRepo: scope._generateImageRepo_(input),
  

•    buildConfig: scope._generateBuildConfig_(input, imageSpec, input.labels),
  

•    deploymentConfig: scope._generateDeploymentConfig_(input, imageSpec, ports, input.labels),
  

•    service: scope._generateService_(input, input.name, scope._getFirstPort_(ports))
  

•  };
  

•  resources.route = scope._generateRoute_(input, input.name, resources.service.metadata.name);
  

•  return resources;
  

• };

• scope.generateRoute = function(input, name, serviceName){

I feel like all these generate functions should really be abstracted
out. Some kind of service that generates JSON for a type based on a set
of inputs. The second we want to create/update individual components we
are going to have to duplicate this.

Significant set of changes at this point, we can do that as a refactor
later.

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/1221/files#r28005808.

Jeff Cantrill
Senior Software Engineer, Red Hat Engineering
Red Hat
Office: 703-748-4420 | 866-546-8970 ext. 8162420
[email protected]
http://www.redhat.com

[jwforres]

Sure an independent create may allow more operations, but the "model"
service or whatever it would be called should in theory handle varied
levels of input, when stuff isn't specified it would take defaults.

I just don't want us duplicating JSON building all over the place if it
isn't necessary, makes dealing with API model changes more troublesome.

On 04/09/2015 02:15 PM, Jeff Cantrill wrote:

In assets/app/scripts/services/applicationGenerator.js
#1221 (comment):

•        return this.name + ":" + this.tag;
  

•      }
  

•    };
  

•  }
  

•  var resources = {
  

•    imageRepo: scope._generateImageRepo_(input),
  

•    buildConfig: scope._generateBuildConfig_(input, imageSpec, input.labels),
  

•    deploymentConfig: scope._generateDeploymentConfig_(input, imageSpec, ports, input.labels),
  

•    service: scope._generateService_(input, input.name, scope._getFirstPort_(ports))
  

•  };
  

•  resources.route = scope._generateRoute_(input, input.name, resources.service.metadata.name);
  

•  return resources;
  

• };

• scope.generateRoute = function(input, name, serviceName){
  I would say that's a strong maybe. Update operations would allow us to
  modify the existing json so I'm not sure it applies. Also, creating
  individual resources I would think we might want to offer a user more
  options then the simplified version of 'Generate Application'.
  … <#>
  On 04/08/2015 03:48 PM, Jessica Forrester wrote: In
  assets/app/scripts/services/applicationGenerator.js
  [DEVEXP-457] Create app from source #1221 (comment):
• return this.name + ":" + this.tag; > + } > + }; > + } > + > + var
  resources = { > + imageRepo: scope.generateImageRepo(input), > +
  buildConfig: scope.generateBuildConfig(input, imageSpec,
  input.labels), > + deploymentConfig:
  scope.generateDeploymentConfig(input, imageSpec, ports,
  input.labels), > + service: scope.generateService(input, input.name,
  scope.getFirstPort(ports)) > + }; > + resources.route =
  scope.generateRoute(input, input.name,
  resources.service.metadata.name); > + return resources; > + }; > + > +
  scope.generateRoute = function(input, name, serviceName){ I feel
  like all these generate functions should really be abstracted out.
  Some kind of service that generates JSON for a type based on a set of
  inputs. The second we want to create/update individual components we
  are going to have to duplicate this. Significant set of changes at
  this point, we can do that as a refactor later. — Reply to this email
  directly or view it on GitHub
  https://github.com/openshift/origin/pull/1221/files#r28005808.
  -- Jeff Cantrill Senior Software Engineer, Red Hat Engineering Red Hat
  Office: 703-748-4420 | 866-546-8970 ext. 8162420 [email protected]
  http://www.redhat.com

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/1221/files#r28087565.

[jwforres]

[merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1519/) (Image: devenv-fedora_1246)

[openshift-bot]

[Test]ing while waiting on the merge queue

[openshift-bot]

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1766/)

[bparees]

woo! awesome work!

[pweil-]

🏆

[jcantrill]

[merge]

[openshift-bot]

Evaluated for origin up to bb87590