> On Aug. 18, 2017, 9:44 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java > > Lines 1312 (patched) > > <https://reviews.apache.org/r/61707/diff/2/?file=1799012#file1799012line1317> > > > > Is it OK if the above if-statement executes and sets the > > `kerberosDescription` from > > `getKerberosDescriptorPreConfigurationFileLocation` so that this if-check > > is always skipped? > > > > If it's OK to skip getting the descriptor if the pre-config descriptor > > is fetched, then drop this.
The logic here is to build the Kerberos descriptor by setting lower priority values before the higher ones. Where as the lower priority items may be overwritten by highter priority ones. In this case, the preconfigure items may be overwritten by the stack defaults, which may be overwritten by user-supplied values. Each of the levels of data are optional, so properly handling is needed to avoid NPEs. The flow first builds a base KerberosDescriptor from the preconfigure data. If it does not exist, the base KerberosDescriptor will be the stack-level KerberosDescriptor. If that does not exist, the base KerberosDescriptor will be the the user-supplied KerberosDescriptor. If a user-suppled KerberosDescriptor is not provided, than and empty KerberosDescriptor should be returned. Meanwhile at any step; if a KerberosDescriptor exists, it will be updated with data from the relevant level-specific KerberosDescriptor. Workflow: - Get the preconfigure data and create the composite KerberosDescriptor; else if there is no preconfigure data, the composite KerberosDescriptor is null - Get the stack-level data and create the stack-level KerberosDescriptor; else if there is no stack-level data, the stack-level KerberosDescriptor is null - If the composite KerberosDescriptor is null, use the stack-level data as the composite; else if the composite KerberosDescriptor exists and the stack-level KerberosDescritor exists, then update the composite KerberosDescriptor with the stack-level KerberosDescriptor. - Get the user-supplied data and create the user-level KerberosDescriptor; else if there is no user-supplied data, the user-level KerberosDescriptor is null - If the composite KerberosDescriptor is null, use the user-level data as the composite; else if the composite KerberosDescriptor exists and the user-level KerberosDescritor exists, then update the composite KerberosDescriptor with the user-level KerberosDescriptor. - If the composite KerberosDescriptor is null, return an empty KerberosDescriptor; else return the composite KerberosDescriptor. Unless I am missing something, the code basically implements this. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61707/#review183290 ----------------------------------------------------------- On Aug. 17, 2017, 8:51 a.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61707/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2017, 8:51 a.m.) > > > Review request for Ambari, Attila Magyar, Balázs Bence Sári, Eugene > Chekanskiy, Jonathan Hurley, Laszlo Puskas, Nate Cole, and Sebastian Toader. > > > Bugs: AMBARI-21602 > https://issues.apache.org/jira/browse/AMBARI-21602 > > > Repository: ambari > > > Description > ------- > > Pre-configure (certain) services when Kerberos is enabled to reduce number of > core service restarts when services are added. > > While processing the Kerberos descriptor, include services marked to be > _pre-configured_. When a tagged service is encountered, process it weather > it is installed or not. However if it is not installed, only apply > configuration changes for existing configuration types. This will set at > least the core-site changes related to proxyuser and auth-to-local rules > properties. By doing this, if a tagged service is later installed, the > settings will already be in place in the existing service configs and thus > the existing services will not need to be restarted. > > Caveats: > - Default values for the uninstalled, tagged, services will be assumed > - The stack advisor will be used to suggest locations of components - used to > build the clusterHostInfo structure that may be used to derive property > values. > > Note: This processing is to occur when Kerberos is enabled. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java > eb97ee376d > > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java > 7ba1b1887d > > ambari-server/src/main/java/org/apache/ambari/server/controller/DeleteIdentityHandler.java > 3329e76226 > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java > 3819863763 > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java > 6c6c43911b > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java > 91a84ea643 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterKerberosDescriptorResourceProvider.java > 59bd96a8f5 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackVersionResourceProvider.java > 64ead405eb > > ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/RemovableIdentities.java > d4bb501231 > > ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/UsedIdentities.java > 46f5642eb1 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractPrepareKerberosServerAction.java > dd2b2237e5 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java > 2e331bb77f > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PreconfigureServiceType.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareDisableKerberosServerAction.java > 60523cdd75 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareEnableKerberosServerAction.java > ca15695564 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareKerberosIdentitiesServerAction.java > f239cffd93 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java > 78aaa77a48 > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java > 23fd0a95f5 > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java > ff1d37808d > ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java > 149579f57d > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptor.java > 7f53daa357 > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptor.java > eba1b3aecc > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosServiceDescriptor.java > 771a23cd3f > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java > 1f10d7e122 > > ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml > e07e28e062 > ambari-server/src/main/resources/stacks/HDP/2.6/kerberos_preconfigure.json > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java > 6be9f328af > > ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java > 5afe87ebb0 > > ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java > b22099973c > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java > e512b66ab0 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterKerberosDescriptorResourceProviderTest.java > f6fc59d511 > > ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleanerTest.java > 027f339eb6 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java > 86f6d3b778 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelperTest.java > 37cfad9c57 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosServiceDescriptorTest.java > 064e1cc328 > > ambari-server/src/test/resources/stacks/HDP/2.0.8/kerberos_preconfigure.json > PRE-CREATION > > > Diff: https://reviews.apache.org/r/61707/diff/2/ > > > Testing > ------- > > Manually tested in cluster > > # Local test results: > ``` > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 40:13 min > [INFO] Finished at: 2017-08-16T15:38:34-04:00 > [INFO] Final Memory: 106M/1721M > [INFO] > ------------------------------------------------------------------------ > ``` > > # Jenkins test results: > > ``` > {color:green}+1 overall{color}. Here are the results of testing the latest > attachment > > http://issues.apache.org/jira/secure/attachment/12882231/AMBARI-21602_trunk_01.patch > against trunk revision . > > {color:green}+1 @author{color}. The patch does not contain any @author > tags. > > {color:green}+1 tests included{color}. The patch appears to include 10 > new or modified test files. > > {color:green}+1 release audit{color}. The applied patch does not > increase the total number of release audit warnings. > > {color:green}+1 javac{color}. The applied patch does not increase the > total number of javac compiler warnings. > > {color:green}+1 core tests{color}. The patch passed unit tests in > ambari-server. > > Console output: > https://builds.apache.org/job/Ambari-trunk-test-patch/12008//console > ``` > > > Thanks, > > Robert Levas > >
