Conversation
|
@VojtechVitek @smarterclayton review pls? all changes should be already there and it should be updated and simplified for the latest JSON. |
pkg/template/template.go
Outdated
There was a problem hiding this comment.
Seems to me like a useless wrapper.
There was a problem hiding this comment.
It is used in tests, it might be used in the cmd tool.
There was a problem hiding this comment.
This is a useless wrapper (it doesn't do enough to justify existence) - use json.Marshal directly in the tests.
mfojtik
force-pushed
the
project_updated
branch
from
2ae4faa to
9b71d15
Compare
|
@smarterclayton thanks, updated! |
mfojtik
force-pushed
the
project_updated
branch
2 times, most recently
from
5eff6ba to
bde0ff7
Compare
pkg/template/generator/template.go
Outdated
There was a problem hiding this comment.
s/GeneratorTemplate/ExpressionValueGenerator/ ?
There was a problem hiding this comment.
I prefer shorter names ;-) It is in fact generator template.
There was a problem hiding this comment.
It still doesn't sound right to me :)
There was a problem hiding this comment.
If it's a value generator that should be part of the name.
pkg/template/template.go
Outdated
There was a problem hiding this comment.
Godoc and justification for it being public?
VojtechVitek
force-pushed
the
project_updated
branch
2 times, most recently
from
4f9fb4a to
00a315f
Compare
|
@smarterclayton refactored & rebased. Review pls. |
There was a problem hiding this comment.
So this will be turned into a replicationcontroller definition as part of the processing into a config file?
There was a problem hiding this comment.
The deployment config controller does that post creation - dconfigs indicate a desire for the described resources to exist but it's their responsibility to create those resources.
There was a problem hiding this comment.
ok, what/where is the deployment config controller?
my current mental model is:
project.json -> env variable generator -> project.json(variables resolved) -> k8s config generator -> config.json(consisting of k8s api objects, basically looks like @mfojtik 's "stapled together objects" config.json) -> calls to k8s apiserver
Where does the deployment config controller fit into that?
There was a problem hiding this comment.
ok, talked to clayton and he got me straightened out: deployment config will be a k8s api object.
There was a problem hiding this comment.
@bparees @smarterclayton so just to clarify, the workflow @bparees proposed above is correct?
I mean, the steps are:
- POST http://localhost:8081/osapi/v1beta1/templates <- project.json -> project_substituted.json
- POST http://localhost:8081/osapi/v1beta1/??? <- project_substituted.json -> config.json
./kubecfg.sh apply config.json
VojtechVitek
force-pushed
the
project_updated
branch
from
00a315f to
654e07e
Compare
VojtechVitek
force-pushed
the
project_updated
branch
from
654e07e to
3f8b50d
Compare
|
@smarterclayton any comments? Merge? |
|
Will take a look before monday |
|
That'd be awesome. Thanks! |
|
code lgtm :-) nice job with refactoring @VojtechVitek! |
|
@smarterclayton @VojtechVitek I just noticed that when we adding custom parameters, we actually appending them to the parameter list, which might be a problem when the parameter with the same name already exists in the template. I added method |
There was a problem hiding this comment.
Can you provide any case this can happen during runtime? The only I can think of is that somebody will remove all generators and then call GenerateParameterValue, which is very unlikely to happen.
There was a problem hiding this comment.
Correct. It's just a check for possible generator initialization failure. We should be safe to remove this, as we have a test case for this.
|
The last commit just improve the test coverage to 93%. |
pkg/template/template.go
Outdated
There was a problem hiding this comment.
Use godoc formatting http://blog.golang.org/godoc-documenting-go-code
mfojtik
force-pushed
the
project_updated
branch
from
40ccf02 to
9fb6c6f
Compare
mfojtik
force-pushed
the
project_updated
branch
from
9fb6c6f to
0988895
Compare
|
@smarterclayton @bparees @VojtechVitek according to the mails we exchanges, I will move this PR to the OpenShift kubernetes fork, refactor the example to look more like a k8s Config and add the REST endpoint to OpenShift k8s. Does it sound OK? |
|
PR moved to k8s: openshift/kubernetes#7 |
Correct json/yaml name for Triggers
…lugin-loader Fix loading openshiftConsoleExtensions by reintroducing hawtioPluginLoader
4 participants
This PR contains an updated version of the Project Template support based on the latest JSON schema.
It is re-using existing Kubernetes structs and the transformation process is independent from the the data structures.