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

promoted group prune and sync from experimental #6369

Merged

Conversation

stevekuznetsov
Copy link
Contributor

ref #6276

This PR promotes openshift ex {sync,prune}-groups to oadm groups {sync,prune}

@deads2k PTAL

@stevekuznetsov
Copy link
Contributor Author

[test][extended:ldap_groups]

@deads2k
Copy link
Contributor

deads2k commented Dec 17, 2015

Link to the doc updates please.

@deads2k
Copy link
Contributor

deads2k commented Dec 17, 2015

Update your description to show where this moved to and from.

@deads2k
Copy link
Contributor

deads2k commented Dec 17, 2015

Also, since we doc'ed it, maintain the location under openshift ex too. Not the go package, just the commands.

@stevekuznetsov
Copy link
Contributor Author

@deads2k doc PR made, re-instated openshift ex {sync,prune}-groups

@deads2k
Copy link
Contributor

deads2k commented Dec 17, 2015

Add test for all four flavors in test-cmd. Just a simple openshift ex sync-groups --help with a text check to make sure we're correctly plumbed through.

@stevekuznetsov
Copy link
Contributor Author

Which bucket should I put that into? help? admin ?

@deads2k
Copy link
Contributor

deads2k commented Dec 17, 2015

put it in help

@stevekuznetsov
Copy link
Contributor Author

[test]

@stevekuznetsov stevekuznetsov force-pushed the skuznets/promote-group-commands branch 2 times, most recently from 0d70520 to f7b2ed8 Compare December 17, 2015 19:45
@stevekuznetsov
Copy link
Contributor Author

again #6259...
[test][extended:ldap_groups]

@stevekuznetsov
Copy link
Contributor Author

flake #6173 here

@stevekuznetsov
Copy link
Contributor Author

re[test][extended:ldap_groups]

@stevekuznetsov
Copy link
Contributor Author

@deads2k finally got green

@deads2k
Copy link
Contributor

deads2k commented Dec 18, 2015

lgtm. @liggitt any complaint on the name?

@0xmichalis
Copy link
Contributor

Shouldn't groups prune be a part of the existing prune command?

@0xmichalis
Copy link
Contributor

Also are these two commands what 6276 is asking for?

@stevekuznetsov
Copy link
Contributor Author

@soltysh #6276 touches this and also another pull AFAIK
oadm prune groups does seem more fitting, but I really don't like the user story for oadm prune groups and oadm groups sync...

@stevekuznetsov
Copy link
Contributor Author

@deads2k should we put prune under oadm prune or oadm groups? Do we have a strategy doc for this?

@deads2k
Copy link
Contributor

deads2k commented Dec 21, 2015

@deads2k should we put prune under oadm prune or oadm groups? Do we have a strategy doc for this?

You'll know it when you see it.

@deads2k
Copy link
Contributor

deads2k commented Dec 21, 2015

@stevekuznetsov cross link prune to support all three calling locations. Add the new location to test-cmd/help

@stevekuznetsov
Copy link
Contributor Author

@deads2k what should doc look like? oadm groups prune? oadm prune groups? Should we even mention the duality?

@deads2k
Copy link
Contributor

deads2k commented Dec 21, 2015

@deads2k what should doc look like? oadm groups prune? oadm prune groups? Should we even mention the duality?

Let's keep congruence, so oadm groups prune. Don't bother mentioning aliases.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 21, 2015
@stevekuznetsov
Copy link
Contributor Author

@deads2k rebased, re[test][extended:ldap_groups]

@stevekuznetsov
Copy link
Contributor Author

@deads2k Added help-text fixes (addition of --confirm to examples, fixed --type instead of --existing) but no renames to sync.go. PTAL

@deads2k
Copy link
Contributor

deads2k commented Dec 21, 2015

@deads2k Added help-text fixes (addition of --confirm to examples, fixed --type instead of --existing) but no renames to sync.go. PTAL

I don't understand. Go back in your git reflog, find the changes and give me a second commit so I can jump to it.

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 21, 2015
$ oadm groups sync --whitelist=/path/to/whitelist.txt --sync-config=/path/to/sync-config.yaml

# Sync all OpenShift Groups that have been synced previously with an LDAP server
$ oadm groups sync --existing --sync-config=/path/to/ldap-sync-config.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

need to regenerate

@deads2k
Copy link
Contributor

deads2k commented Dec 21, 2015

I don't understand. Go back in your git reflog, find the changes and give me a second commit so I can jump to it.

I think I understand now. Get yourself back to green.

@stevekuznetsov
Copy link
Contributor Author

updated docs & completions, re[test][extended:ldap_groups]

@stevekuznetsov
Copy link
Contributor Author

e2e failing dc succeeded here:

deploymentconfig "failing-dc" created
++ Retrying until success or timeout: oc get rc/failing-dc-1

[FAIL] !!!!! Test Failed !!!!

may be #5680 ? output looks different

@stevekuznetsov
Copy link
Contributor Author

re[test][extended:ldap_groups]

@stevekuznetsov
Copy link
Contributor Author

flake on e2e pushing image here:

[INFO] Tagging and pushing ruby-22-centos7 to 172.30.83.160:5000/cache/ruby-22-centos7:latest
...
Error from server: User "e2e-user" cannot list all buildconfigs in the cluster

#6149

re[test][extended:ldap_groups]

@stevekuznetsov
Copy link
Contributor Author

flake on e2e here:

Error from server: User "e2e-user" cannot list all buildconfigs in the cluster

#6149

@liggitt
Copy link
Contributor

liggitt commented Dec 22, 2015

I think the real error is:

Error from server: 501: All the given peers are not reachable (failed to propose on members [https://172.18.7.165:4001] twice [last error: Unexpected HTTP status code]) [0]
!!! Error in hack/../test/end-to-end/core.sh:160
    'oc delete pod cli-with-token' exited with status 1
Call stack:
    1: hack/../test/end-to-end/core.sh:160 main(...)
Exiting with status 1

@stevekuznetsov
Copy link
Contributor Author

@liggitt That seems more likely. Any opposition to me refactoring e2e to use os::cmd so we can read the output?

@liggitt
Copy link
Contributor

liggitt commented Dec 22, 2015

Any opposition to me refactoring e2e to use os::cmd so we can read the output?

No strong opposition... I'd maybe start with just the ones capturing/echoing output to make the review easy, since that's the main issue that is making our lives difficult at the moment

@stevekuznetsov
Copy link
Contributor Author

re[test][extended:ldap_groups]

@stevekuznetsov
Copy link
Contributor Author

TestAuthorizationSubjectAccessReview flaked here:

I1222 11:46:52.522839   20535 etcd.go:37] Deleting &etcd.Node{Key:"/kubernetes.io", Value:"", Dir:true, Expiration:(*time.Time)(nil), TTL:0, Nodes:etcd.Nodes(nil), ModifiedIndex:0x33e, CreatedIndex:0x33e} (child of &etcd.Node{Key:"", Value:"", Dir:true, Expiration:(*time.Time)(nil), TTL:0, Nodes:etcd.Nodes{(*etcd.Node)(0xc2085e8060), (*etcd.Node)(0xc2085e80c0)}, ModifiedIndex:0x0, CreatedIndex:0x0})
F1222 11:46:58.802390   20535 etcd.go:39] Unable to delete key: 100: Key not found (/kubernetes.io) [1113]

#6065
re[test][extended:ldap_groups]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 6eb1b36

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/8081/) (Extended Tests: ldap_groups)

@deads2k
Copy link
Contributor

deads2k commented Dec 22, 2015

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4463/) (Image: devenv-rhel7_3022)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 6eb1b36

openshift-bot pushed a commit that referenced this pull request Dec 22, 2015
@openshift-bot openshift-bot merged commit da443e7 into openshift:master Dec 22, 2015
@stevekuznetsov stevekuznetsov deleted the skuznets/promote-group-commands branch January 8, 2016 15:22
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.

5 participants