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

support deployment of replica KDCs incl. kprop sync script and new molecule test scenario #22

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

lhoss
Copy link

@lhoss lhoss commented May 15, 2019

Fixes #21 .

EDIT: Status: Implemented and tested

@NadOby
Copy link

NadOby commented Jun 17, 2019

Hi @lhoss Can you please take a look on the PR, and make it pass CI please.
The problem is lint failure in molecule check

@lhoss
Copy link
Author

lhoss commented Jun 20, 2019

The problem is lint failure in molecule check

I just fixed the lint issues in my changes .. but now there are 4 new lint issues on existing code (maybe due to some newer pip module ?!)
Those should be solved in a different PR cc @HorizonNet .

Anyway I'll now continue work on this PR, mainly testing if it works with a replica kdc 👍

@NadOby
Copy link

NadOby commented Jun 20, 2019

@lhoss I opened PR to your rep with all fixes implemented so you can just pick
scigility#1
There are changes in Travic CI, molecule compatibility, and moved from python 2.7 to 3.6

@lhoss
Copy link
Author

lhoss commented Jun 20, 2019

@NadOby great work thx (and so quick)!
To get some more order, I propose to separate your fixes in separate PR, doing following:

  • you open a PR to the upstream ultratendency repo, based on master (Adv.: it has travis enabled!)
    • or we can work on our scigility repo, then I'ld rebase your PR to master.. and then I merge it..
  • and I'll rebase my (WIP!) branch on your work
    wdyt ?

@NadOby
Copy link

NadOby commented Jun 20, 2019

I opened PR to your repo.
The proposition is to do all work in your scigility repo, cause major of work was done inside it, and then update ultratendency one.

@lhoss lhoss closed this Jun 25, 2019
@lhoss lhoss reopened this Jun 25, 2019
@lhoss lhoss changed the title WIP: support for multiple KDCs (untested) support deployment of replica KDCs incl. kprop sync script and new molecule test scenario Jun 25, 2019
@lhoss
Copy link
Author

lhoss commented Jun 25, 2019

This PR is now ready for review 👍

@NadOby
Copy link

NadOby commented Jul 4, 2019

Hi, @lhoss I'm gone over the PR and there are errors when running molecule test on the default scenario name.
molecule test
You don't see it because it is commented out in .travis.yaml and rely on it to effectively check code validity.
So you can uncomment it in .travis.yaml
Excepting this small issue, all seems good.

Copy link
Member

@HorizonNet HorizonNet left a comment

Choose a reason for hiding this comment

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

LGTM overall. Love the change. Let's try to bring it in. We have some people also reviewing it.

.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
molecule/kdc-with-replica/molecule.yml Show resolved Hide resolved
molecule/kdc-with-replica/molecule.yml Show resolved Hide resolved
molecule/default/molecule.yml Outdated Show resolved Hide resolved
molecule/kdc-with-replica/molecule.yml Outdated Show resolved Hide resolved
tasks/redhat.yml Outdated Show resolved Hide resolved
tasks/kprop_replication.yml Show resolved Hide resolved
tasks/kprop_replication.yml Show resolved Hide resolved
tasks/kprop_replication.yml Show resolved Hide resolved
@HorizonNet
Copy link
Member

We should also create a new version after merging this. In preparation I tagged the current master as v1.0.0. After this one I would create a v2.0.0.

@HorizonNet
Copy link
Member

@lhoss Do we also need to raise the minimum Ansible version?

@lhoss
Copy link
Author

lhoss commented Jul 17, 2019

@lhoss Do we also need to raise the minimum Ansible version?

Yes ansible 2 is a more than a must, but I recommend to put >2.5 (I ran a test with 2.5.8 and that worked 👍 )

@lhoss
Copy link
Author

lhoss commented Jul 17, 2019

@HorizonNet I fixed all your (good!) review comments.
Now (after lunch) I want to (force)push the latest state, rebased on #23 improvements, which will also make both molecule test scenarios (the simple/default one, and the kdc-with-replica) work. I hope you do not mind (github might not show the comment history after the force push)
After that I also want to push a small improvement in the add_princ_and_kt.yml, making it flexibler to re-use that outside the role (plus a keytab-based kinit 1-task include, which I often need in playbooks, and which is not worth to put into a separate role, but I like to abstract it out anyway 👍 )

@lhoss
Copy link
Author

lhoss commented Jul 17, 2019

ps: @HorizonNet wdyt about this new kinit checks feature, that I created in a derived branch?!
scigility/kerberos_server@support_multiple_kdcs...scigility:support_multiple_kdcs-with_kinit

For me it is useful to re-use in other playbooks, and to avoid to duplicate the logic (even though it is only 1 task , I know). Here an example with import_role:

  tasks:
    - import_role:
        name: ../../kerberos_server
        #name: kerberos_server
        tasks_from: kinit_kt
      vars:
        # ensure kerberos vars are set, that align with kerberos_server vars !
        _realm_name: "{{ kerberos_server_realm_name }}"
        _principal: "{{ hdfs_root_user }}/{{ ansible_fqdn }}"
        _keytab: "{{ hdfs_keytab }}"
        _kinit_user: "{{ hdfs_root_user }}"
      when: do_kinit | default(True) | bool
      tags: kinit

…a_sync.sh, todo push molecule 2-node test scenario)
…tch + copy), because synchronize +delegate_to +'running in docker' (molecule tests) failed for me
Copy link
Member

@HorizonNet HorizonNet left a comment

Choose a reason for hiding this comment

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

LGTM. There is a merge conflict due to the merge of #23. Could you please resolve it? Then we're going to bring this in finally. I'll then create a new version.

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.

Support the installation of Replica KDCs
3 participants