Setting deployment and build triggers via CLI #7423
Conversation
smarterclayton
force-pushed
the
set_strategy
branch
2 times, most recently
from
dabda1e to
f8e62cc
Compare
smarterclayton
force-pushed
the
set_strategy
branch
from
f8e62cc to
9d9355e
Compare
smarterclayton
changed the title
|
[test] |
smarterclayton
force-pushed
the
set_strategy
branch
3 times, most recently
from
997aab8 to
c5d6aff
Compare
|
This is on the 1.2 target, if we can get some eyes on it today I'd appreciate it. |
|
Will get to it later today |
| # Remove all triggers | ||
| $ %[1]s triggers bc/webapp --remove-all | ||
|
|
||
| # Stop triggering on config change |
smarterclayton
force-pushed
the
set_strategy
branch
2 times, most recently
from
e55e95b to
d83ae39
Compare
|
More comments needed... :) |
| Selector string | ||
| All bool | ||
|
|
||
| Builder *resource.Builder |
There was a problem hiding this comment.
Since you pass Infos, you don't really need the resource builder past Complete. Resolve all Infos in Complete.
There was a problem hiding this comment.
openshift-bot
added
the
needs-rebase
| return t | ||
| } | ||
|
|
||
| func (t *TriggerDefinition) Apply(obj runtime.Object) error { |
There was a problem hiding this comment.
this seems like it could use some direct unit testing. i'm especially worried about the strategyTrigger matching logic either being broken now, or getting broken in the future.
smarterclayton
force-pushed
the
set_strategy
branch
from
561c69d to
40d7b44
Compare
openshift-bot
removed
the
needs-rebase
|
Turned |
smarterclayton
force-pushed
the
set_strategy
branch
from
a0a9019 to
ba516be
Compare
|
Any more comments? |
smarterclayton
force-pushed
the
set_strategy
branch
from
ba516be to
c5873e6
Compare
|
@smarterclayton LGTM |
`oc set triggers` will display, remove, update, or alter triggers on deployment configs and build configs.
smarterclayton
force-pushed
the
set_strategy
branch
from
c5873e6 to
304a2b2
Compare
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5112/) (Image: devenv-rhel7_3548) |
|
Evaluated for origin merge up to 304a2b2 |
|
[test] |
2 similar comments
|
[test] |
|
[test] |
|
Evaluated for origin test up to 304a2b2 |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/1647/) |
Merged by openshift-bot
| url=":${API_PORT:-8443}" | ||
| project="$(oc project -q)" | ||
|
|
||
| # This test validates builds and build related commands |
There was a problem hiding this comment.
@smarterclayton I'm assuming this is copy-pasta error?
There was a problem hiding this comment.
Yes
On Fri, Mar 11, 2016 at 3:07 PM, Steve Kuznetsov notifications@github.com
wrote:
In test/cmd/triggers.sh
#7423 (comment):+source "${OS_ROOT}/hack/util.sh"
+source "${OS_ROOT}/hack/cmd_util.sh"
+os::log::install_errexit
+
+# Cleanup cluster resources created by this test
+(
- set +e
- oc delete all --all
- exit 0
+) &>/dev/null
+url=":${API_PORT:-8443}"
+project="$(oc project -q)"
+
+# This test validates builds and build related commands@smarterclayton https://github.com/smarterclayton I'm assuming this is
copy-pasta error?—
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/7423/files#r55881699.
Exposes a command for managing all triggers on an object, which today are build configs and deployment configs. Provides a consistent experience across both (even though the API is inconsistent). Highlights that we should add web hooks to deployments.
Usage is
oc set triggers dc/footo print the triggers, thenoc set triggers dc/foo --manualto set them to manual,oc set triggers dc/foo --autoto set them on,oc set triggers dc/foo --from-image=.... -c ...to set a trigger on particular set of containers, and then other flags like--from-webhook=to create a new web hook secrets.