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

DRY out script cleanup code #14116

Merged
merged 3 commits into from May 13, 2017

Conversation

stevekuznetsov
Copy link
Contributor

Disable swap in Go build

We have migrated to Go 1.7+ and as such we no longer need to be enabling
swap space for our builds.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


Automatically determine which type of jUnit report to generate

We can tell from the raw test output file which type of test was used to
create the output, so we do not need users to have to specify which type
of parsing and report they would like.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


DRY out script cleanup code

Almost every single cleanup function trapped on exit for our scripts was
taking the same actions in the same order. We can break that out into a
standalone function under hack/lib that contains a superset of all the
cleanup steps used, provided that we make sure each individual clean up
step can gracefully handle the case where it is not needed or does not
have any work to do.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


/cc @smarterclayton PTAL

[test]

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2017
@stevekuznetsov stevekuznetsov force-pushed the skuznets/cleanup_all branch 5 times, most recently from 8c1675d to f8940f0 Compare May 9, 2017 20:57
@stevekuznetsov stevekuznetsov removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2017
@smarterclayton
Copy link
Contributor

smarterclayton commented May 9, 2017 via email

@stevekuznetsov stevekuznetsov force-pushed the skuznets/cleanup_all branch 7 times, most recently from b8c8683 to ed72c67 Compare May 10, 2017 19:35
We have migrated to Go 1.7+ and as such we no longer need to be enabling
swap space for our builds.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
We can tell from the raw test output file which type of test was used to
create the output, so we do not need users to have to specify which type
of parsing and report they would like.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Almost every single cleanup function trapped on exit for our scripts was
taking the same actions in the same order. We can break that out into a
standalone function under `hack/lib` that contains a superset of all the
cleanup steps used, provided that we make sure each individual clean up
step can gracefully handle the case where it is not needed or does not
have any work to do.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
@stevekuznetsov
Copy link
Contributor Author

[test][merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 8c7f1a7

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 8c7f1a7

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1374/) (Base Commit: 012de50)

@openshift-bot
Copy link
Contributor

openshift-bot commented May 13, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/625/) (Base Commit: a2e505b) (Image: devenv-rhel7_6228)

@smarterclayton
Copy link
Contributor

This kills my docker containers when i run hack/update-generated-swagger-spec.sh, which is annoying:

SUCCESS after 2.964s: hack/lib/start.sh:298: executing 'oc get --raw /healthz --config='/var/folders/zw/j6656mbn1rv4fnqqdggss8h80000gn/T//openshift/update-generated-swagger-spec/openshift.local.config/master/admin.kubeconfig'' expecting any result and text 'ok'; re-trying every 0.25s until completion or 160.000s
SUCCESS after 0.240s: hack/lib/start.sh:299: executing 'oc get --raw /healthz/ready --config='/var/folders/zw/j6656mbn1rv4fnqqdggss8h80000gn/T//openshift/update-generated-swagger-spec/openshift.local.config/master/admin.kubeconfig'' expecting any result and text 'ok'; re-trying every 0.25s until completion or 160.000s
SUCCESS after 0.248s: hack/lib/start.sh:300: executing 'oc get service kubernetes --namespace default --config='/var/folders/zw/j6656mbn1rv4fnqqdggss8h80000gn/T//openshift/update-generated-swagger-spec/openshift.local.config/master/admin.kubeconfig'' expecting success; re-trying every 0.25s until completion or 160.000s
[INFO] Updating /Volumes/development/projects/origin/src/github.com/openshift/origin//api/swagger-spec:
[INFO] Updating /api/swagger-spec/oapi-v1.json from /swaggerapi/oapi/v1...
[INFO] Updating /api/swagger-spec/api-v1.json from /swaggerapi/api/v1...
[INFO] [CLEANUP] Beginning cleanup routines...
[INFO] [CLEANUP] Dumping cluster events to _output/scripts/update-generated-swagger-spec/artifacts/events.txt
[INFO] [CLEANUP] Dumping etcd contents to _output/scripts/update-generated-swagger-spec/artifacts/etcd
2017-05-21 23:32:40.029884 I | warning: ignoring ServerName for user-provided CA for backwards compatibility is deprecated
[INFO] [CLEANUP] Dumping container logs to _output/scripts/update-generated-swagger-spec/logs/containers
[INFO] [CLEANUP] `pprof` output logged to _output/scripts/update-generated-swagger-spec/logs/pprof.out
[INFO] [CLEANUP] Truncating log files over 100M
[INFO] [CLEANUP] Stopping docker containers
[INFO] [CLEANUP] Removing docker containers
[INFO] [CLEANUP] Killing child processes
[INFO] [CLEANUP] Pruning etcd data directory
[INFO] hack/update-generated-swagger-spec.sh exited with code 0 after 00h 00m 15s

update-generated-swagger-spec has nothing to do with docker containers or etcd data directory.

@stevekuznetsov
Copy link
Contributor Author

Since we start a server for Swagger anyway I'm pretty sure container cleanup was pre-existing. We haven't changed the logic for which containers we target for cleanup. Which ones are you seeing killed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants