You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Eugene Chekanskiy <ec...@hortonworks.com> on 2017/01/11 13:32:17 UTC
Review Request 55424: Stack Downgrading Potentially Corrupts Kerberos
Descriptor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/
-----------------------------------------------------------
Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
Bugs: AMBARI-19464
https://issues.apache.org/jira/browse/AMBARI-19464
Repository: ambari
Description
-------
AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/55424/diff/
Testing
-------
mvn clean test
Thanks,
Eugene Chekanskiy
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Dmytro Grinenko <ha...@gmail.com>.
> On Jan. 11, 2017, 1:48 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java, lines 246-248
> > <https://reviews.apache.org/r/55424/diff/1/?file=1602491#file1602491line246>
> >
> > I'm worried about this part. If something goes wrong on the downgrade in the next method and the user re-tries the command, it won't be able to find the backup, right?
> >
> > I know you've already restored it - but it's good to keep the backup just in case you need it.
>
> Robert Levas wrote:
> I think that I am indifferent about this, but it can't hurt to leave the data around. If we leave it around, would it be usefil to append the relevant stack version to it? For example: `kerberos_descriptor_backup_2.5`
well, i'm not fan of making a backup storage from Ambari Database. In general, ttl for descriptor is from one stack version to another or forced regeneration. That means, that after sucessfull downgrade, that backup would not be used anymore and will just live with Ambari next 3 years.
I seen the code above, that we will remove previous backup if found one. That could be enough and here we could just keep current backup, so user can re-try as much as he wants. This should solve problem
> On Jan. 11, 2017, 1:48 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java, line 124
> > <https://reviews.apache.org/r/55424/diff/1/?file=1602491#file1602491line124>
> >
> > Kind of a double-negative - can you make this if(isUpgrade()) instead?
or just swap content of if...else ... endif block and remove one negative "!"
- Dmytro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161228
-----------------------------------------------------------
On Jan. 11, 2017, 1:32 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 1:32 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Robert Levas <rl...@hortonworks.com>.
> On Jan. 11, 2017, 8:48 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java, lines 246-248
> > <https://reviews.apache.org/r/55424/diff/1/?file=1602491#file1602491line246>
> >
> > I'm worried about this part. If something goes wrong on the downgrade in the next method and the user re-tries the command, it won't be able to find the backup, right?
> >
> > I know you've already restored it - but it's good to keep the backup just in case you need it.
I think that I am indifferent about this, but it can't hurt to leave the data around. If we leave it around, would it be usefil to append the relevant stack version to it? For example: `kerberos_descriptor_backup_2.5`
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161228
-----------------------------------------------------------
On Jan. 11, 2017, 8:32 a.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 8:32 a.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161228
-----------------------------------------------------------
Fix it, then Ship it!
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 123)
<https://reviews.apache.org/r/55424/#comment232471>
Kind of a double-negative - can you make this if(isUpgrade()) instead?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 213)
<https://reviews.apache.org/r/55424/#comment232472>
Documentation.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 219)
<https://reviews.apache.org/r/55424/#comment232473>
Null Check and throw exception?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 230)
<https://reviews.apache.org/r/55424/#comment232474>
Documentation.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (lines 245 - 247)
<https://reviews.apache.org/r/55424/#comment232475>
I'm worried about this part. If something goes wrong on the downgrade in the next method and the user re-tries the command, it won't be able to find the backup, right?
I know you've already restored it - but it's good to keep the backup just in case you need it.
- Jonathan Hurley
On Jan. 11, 2017, 8:32 a.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 8:32 a.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Dmytro Grinenko <ha...@gmail.com>.
> On Jan. 11, 2017, 2:30 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java, lines 241-242
> > <https://reviews.apache.org/r/55424/diff/1/?file=1602491#file1602491line241>
> >
> > If backupEntity is `null`, this will generate an NPE
and even more, "artifactDAO.remove(entity);" shouldn't be removed, if there no backup entity available.
- Dmytro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161238
-----------------------------------------------------------
On Jan. 11, 2017, 1:32 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 1:32 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161238
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (lines 130 - 169)
<https://reviews.apache.org/r/55424/#comment232483>
This is only necessary on upgrade. In the downgrade scenario, the backup will used.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (lines 240 - 241)
<https://reviews.apache.org/r/55424/#comment232485>
If backupEntity is `null`, this will generate an NPE
- Robert Levas
On Jan. 11, 2017, 8:32 a.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 8:32 a.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Dmytro Grinenko <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161375
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 123)
<https://reviews.apache.org/r/55424/#comment232620>
or just swap content of if...else ... endif block and remove one negative "!"
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (lines 245 - 247)
<https://reviews.apache.org/r/55424/#comment232621>
well, i'm not fan of making a backup storage from Ambari Database. In general, ttl for descriptor is from one stack version to another or forced regeneration. That means, that after sucessfull downgrade, that backup would not be used anymore and will just live with Ambari next 3 years.
I seen the code above, that we will remove previous backup if found one. That could be enough and here we could just keep current backup, so user can re-try as much as he wants. This should solve problem.
- Dmytro Grinenko
On Jan. 11, 2017, 1:32 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 1:32 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Dmytro Grinenko <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161688
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 252)
<https://reviews.apache.org/r/55424/#comment233001>
should be this an atomic operation?
- Dmytro Grinenko
On Jan. 12, 2017, 2:09 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2017, 2:09 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Dmytro Grinenko <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/#review161689
-----------------------------------------------------------
Ship it!
Ship It!
- Dmytro Grinenko
On Jan. 12, 2017, 2:09 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55424/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2017, 2:09 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
>
>
> Bugs: AMBARI-19464
> https://issues.apache.org/jira/browse/AMBARI-19464
>
>
> Repository: ambari
>
>
> Description
> -------
>
> AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
> However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55424/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 55424: Stack Downgrading Potentially Corrupts
Kerberos Descriptor
Posted by Eugene Chekanskiy <ec...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55424/
-----------------------------------------------------------
(Updated Jan. 12, 2017, 2:09 p.m.)
Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas.
Bugs: AMBARI-19464
https://issues.apache.org/jira/browse/AMBARI-19464
Repository: ambari
Description
-------
AMBARI-18335 addresses an issue where configuration properties in the Kerberos descriptor don't change during a stack upgrade. It accounts for both upgrade/downgrade scenarios and situations where properties are added/removed.
However, it does not take into account the potential for a property to be customized by the user (not a new property, but a property which the user changes the value of). In this case, the downgrade would be destructive, overriding the user's values with those from the downgrade stack.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java 8972e6d
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java f1eab38
ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/55424/diff/
Testing
-------
mvn clean test
Thanks,
Eugene Chekanskiy