----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34823/#review85803 -----------------------------------------------------------
Ship it! Looks good. Some minor comments. ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java <https://reviews.apache.org/r/34823/#comment137607> unused import ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java <https://reviews.apache.org/r/34823/#comment137623> Is this a mistake? I don't see a method KerberosDescriptor#replaceVariables. It seems like VariableReplacementHelper#replaceVariables is valid. ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java <https://reviews.apache.org/r/34823/#comment137626> Looks like field is not used. ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java <https://reviews.apache.org/r/34823/#comment137618> This looks like its only ever called from within this impl class, but it is part of the interface. Was that intentional? ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractPrepareKerberosServerAction.java <https://reviews.apache.org/r/34823/#comment137604> unused import - Tom Beerbower On May 29, 2015, 7:34 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34823/ > ----------------------------------------------------------- > > (Updated May 29, 2015, 7:34 p.m.) > > > Review request for Ambari, Emil Anca, John Speidel, Robert Nettleton, and Tom > Beerbower. > > > Bugs: AMBARI-11396 > https://issues.apache.org/jira/browse/AMBARI-11396 > > > Repository: ambari > > > Description > ------- > > Precondition: On the cluster where this was observed, there were previous > attempts to kerberize the cluster. > > STR: > Go through the Enable Kerberos Wizard. > The only non-default option taken was to not manage krb5.conf (presented on > the second page of the wizard). > > Chrome developer tool shows that there was a POST on > /api/v1/clusters/woah/artifacts/kerberos_descriptor failing with 409. > > ``` > { > "status" : 409, > "message" : "Attempted to create an artifact which already exists, > artifact_name='kerberos_descriptor', > foreign_keys='{Artifacts/cluster_name=woah}'" > } > ``` > > It doesn't seem like this is the cause of the issue (though we need to > investigate). > > The UI keeps showing a spinner for several minutes, then shows a failure. > This is because a call to PUT on the cluster resource to set security_type > takes more than 3 minutes, and the browser aborts the request. > However, the backend kept moving forward to Kerberize the cluster > (ambari-server.log was being tailed to check on progress). > After verifying that all principals and keytabs were generated/distributed, > the wizard was closed (the last step of the wizard is to start all services > and run service checks, but this was skipped because the previous step > failed.) > The cluster was in fact successfully Kerberized. > > _Note:_ The condition is likely to have occurred due to a timeout related to > the number of hosts and services in the cluster. The preparation phase of > enabling Kerberos is performed within the handler of the relavant API call. > Most of this work should be moved out to a stage which is handled > asynchronously. > > #Solution > Speed up the time it takes to generate the stages needed to enable Kerberos. > > This entails moving the bulk of the processing to a new (preparation) stage. > Because of this, `org.apache.ambari.server.controller.KerberosHelper` should > be changed to an interface and the implementation moved to > `org.apache.ambari.server.controller.KerberosHelperImpl`. To provide the > logic for the preparation stages, a set of new classes were are needed, > `org.apache.ambari.server.serveraction.kerberos.Prepare*KerberosServerAction` > which rely on the `KerberosHelper` and > `org.apache.ambari.server.serveraction.kerberos.AbstractPrepareKerberosServerAction` > for shared code. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java > 2474c3d > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > 72c33bd > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java > 141803b > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractPrepareKerberosServerAction.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java > 34780d6 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java > 55018de > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareDisableKerberosServerAction.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareEnableKerberosServerAction.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareKerberosIdentitiesServerAction.java > PRE-CREATION > ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java > 10204ea > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > 186963f > > ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java > 5a6ddd3 > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java > 409b955 > > Diff: https://reviews.apache.org/r/34823/diff/ > > > Testing > ------- > > Manually tested enabling and disabling Kerberos, regenerating keytabs, adding > services, adding hosts. > > #Updated unit tests: > Running org.apache.ambari.server.controller.KerberosHelperTest > Tests run: 42, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.616 sec > > Running org.apache.ambari.server.state.cluster.ClusterTest > Tests run: 28, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 62.714 sec > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >
