search

RedHatSatellite / satellite-clone

Utility to assist cloning an existing Satellite 6 server to a new host
GNU General Public License v3.0
37 stars 31 forks source link

Support cloning satellite 6.8 #379

Closed anandkumaragrawal closed 3 years ago

chris1984 commented 3 years ago

Starting to review

sthirugn commented 3 years ago

LGTM. I don't have the capacity to test, so perhaps need to wait for @chris1984 review and/or test here.

chris1984 commented 3 years ago

Yeah getting the infra setup to support 6.8 and will test then, should have it done today or tomorrow

mccun934 commented 3 years ago

thanks @anandkumaragrawal for the PR!

I tested this with a 6.8 backup set and it bailed here:

TASK [satellite-clone : Get candlepin password] **************************************************************************************************************
Wednesday 02 December 2020  16:48:31 -0500 (0:00:03.335)       0:00:20.143 **** 
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!"}

which is indeed empty in the answers file:

# grep candlepin_db_password etc/foreman-installer/scenarios.d/satellite-answers.yaml 
  candlepin_db_password:

This appears to be a change in the answer file format on 6.8 from the installer. Is this value needed by Satellite Clone?

anandkumaragrawal commented 3 years ago

thanks @anandkumaragrawal for the PR!

I tested this with a 6.8 backup set and it bailed here:

TASK [satellite-clone : Get candlepin password] **************************************************************************************************************
Wednesday 02 December 2020  16:48:31 -0500 (0:00:03.335)       0:00:20.143 **** 
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!"}

which is indeed empty in the answers file:

# grep candlepin_db_password etc/foreman-installer/scenarios.d/satellite-answers.yaml 
  candlepin_db_password:

This appears to be a change in the answer file format on 6.8 from the installer. Is this value needed by Satellite Clone?

@mccun934 I have checked the configuration file in the answers file and that looks good to me. I have tried the clone multiple times and did not fail at all

grep candlepin_db_password /etc/foreman-installer/scenarios.d/satellite-answers.yaml candlepin_db_password: Zpo3y6RcMyBwZzU6JQs9UJ2LSPaSyLwW

bbuckingham commented 3 years ago

@anandkumaragrawal , Hi, Thank you for contributing this PR and all of the included changes!

Any updates on the PR based upon the most recent comments? Are the changes to remove the older versions of Satellite going to be rolled back and a separate issue created to track them?

QE is also looking to get the changes to support cloning 6.8 ASAP to assist with their testing.

johnpmitsch commented 3 years ago

Trying to follow along, what is the scope of this PR now?

If we are removing 6.4 and earlier, we should do that completely. If nothing in this PR broke 6.4 and earlier and there were pre-existing issues, I think that would be a much better separate change so we can ensure it's completeness and test older versions.

johnpmitsch commented 3 years ago

@anandkumaragrawal After some discussion, my understanding is that the issues brought up with the puppet_version function are pre-existing, so lets keep the scope of this to adding 6.8 support and leave removing <6.4 to a separate issue.

This means that the get_puppet_version function can go back and include your original version addition and the sp_features_table_name variable needs to be set for 6.4 and earlier. Once you do that, I can run the automation for this PR.

anandkumaragrawal commented 3 years ago

@johnpmitsch sure, I will get this fixed this weekend so that Monday we will get things working

anandkumaragrawal commented 3 years ago

Modified the changes as requested

mccun934 commented 3 years ago

Tested this out with the latest updates and it looks like we now unnecessarily require the candlepin_db_password in the answers file:


TASK [satellite-clone : Get candlepin password] **************************************************************************************************************
Sunday 20 December 2020  11:01:18 -0500 (0:00:03.324)       0:00:23.428 ******* 
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!"}

msg: Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!

@johnpmitsch I'm guessing this is because of changes we added to support external databases?

foreman-maintain does not include the DB passwords on embedded database installs so we likely don't need to require this during a clone

anandkumaragrawal commented 3 years ago

Tested this out with the latest updates and it looks like we now unnecessarily require the candlepin_db_password in the answers file:


TASK [satellite-clone : Get candlepin password] **************************************************************************************************************
Sunday 20 December 2020  11:01:18 -0500 (0:00:03.324)       0:00:23.428 ******* 
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!"}

msg: Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!

@johnpmitsch I'm guessing this is because of changes we added to support external databases?

foreman-maintain does not include the DB passwords on embedded database installs so we likely don't need to require this during a clone

@mccun934 : I have tried the satellite-clone many a times, but this never failed me. However, if you confirm, I can definetely remove / Comment out this.

johnpmitsch commented 3 years ago

Tested this out with the latest updates and it looks like we now unnecessarily require the candlepin_db_password in the answers file:


TASK [satellite-clone : Get candlepin password] **************************************************************************************************************
Sunday 20 December 2020  11:01:18 -0500 (0:00:03.324)       0:00:23.428 ******* 
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!"}

msg: Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!

@johnpmitsch I'm guessing this is because of changes we added to support external databases?

foreman-maintain does not include the DB passwords on embedded database installs so we likely don't need to require this during a clone

@mccun934 this was a semi-recent addition to support external databases, but not introduced in this PR, @anandkumaragrawal it may be an issue we need to deal with, but no need to make changes here IMO since this PR didn't introduce this functionality.

@mccun934 What version of satellite-clone? The assumption is those values are in satellite-answers, which I thought was standard for all installs. It's been working fine for automation which has an embedded database. We could more dynamically handle those values, I'm trying to understand the scenario where they wouldn't be present.

johnpmitsch commented 3 years ago

Changes look good, running automation now :robot:

johnpmitsch commented 3 years ago

Tested this out with the latest updates and it looks like we now unnecessarily require the candlepin_db_password in the answers file:


TASK [satellite-clone : Get candlepin password] **************************************************************************************************************
Sunday 20 December 2020  11:01:18 -0500 (0:00:03.324)       0:00:23.428 ******* 
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!"}

msg: Keys ['katello', 'candlepin_db_password'] were not found in etc/foreman-installer/scenarios.d/satellite-answers.yaml!

@johnpmitsch I'm guessing this is because of changes we added to support external databases?

foreman-maintain does not include the DB passwords on embedded database installs so we likely don't need to require this during a clone

So automation actually hit the same for 6.8 backups, and doesn't for 6.7. It happens when candlepin_db_password is null. Either this has always been allowed and we aren't handling it (very possible) or something changed in the installer.

We'll need to update the ansible module to accept a allow_null parameter and default to an empty string.

Let me see if I can get a quick PR up for that.

johnpmitsch commented 3 years ago

We'll need to update the ansible module to accept a allow_null parameter and default to an empty string.

Let me see if I can get a quick PR up for that.

Opened up a PR here https://github.com/RedHatSatellite/satellite-clone/pull/380 and based it on top of this PR so it can be tested on 6.8. Running the automation again

johnpmitsch commented 3 years ago

@anandkumaragrawal once I got past the missing candlepin pw with #380, it's failing with:

2020-12-22 19:41:53,864 p=4874 u=root n=ansible | TASK [satellite-clone : Restart katello-service] *******************************
2020-12-22 19:41:53,865 p=4874 u=root n=ansible | Tuesday 22 December 2020  19:41:53 +0000 (0:00:00.669)       0:29:33.070 ******
2020-12-22 19:41:54,637 p=4874 u=root n=ansible | fatal: [localhost]: FAILED! => {"changed": false, "cmd": "'satellite-maintain service' start", "msg": "[Errno 2] No such file or directory", "rc": 2}

Looks like there is a quoting issue with the satellite-maintain service command

anandkumaragrawal commented 3 years ago

@johnpmitsch, It is always a successful run at my end without code change

Perform task: TASK: satellite-clone : Restart katello-service (N)o/(y)es/(c)ontinue: ****************************************************************************************
Perform task: TASK: satellite-clone : Wait for foreman-tasks service to start (N)o/(y)es/(c)ontinue: y

Perform task: TASK: satellite-clone : Wait for foreman-tasks service to start (N)o/(y)es/(c)ontinue: ************************************************************************

TASK [satellite-clone : Wait for foreman-tasks service to start] ************************************************************************************************************
Wednesday 23 December 2020  06:35:47 -0500 (0:00:35.209)       0:00:35.209 **** 
changed: [localhost]

cmd: sleep 300

start: 2020-12-23 06:35:47.663969

end: 2020-12-23 06:40:47.670620

delta: 0:05:00.006651

Also It works fine, If I modify the command to command: " {{ satellite_service }} start"

johnpmitsch commented 3 years ago

Also It works fine, If I modify the command to command: " {{ satellite_service }} start"

@anandkumaragrawal we'll need this change made here.

I made the above change and also changed the default satellite_service value to satellite-maintain service in https://github.com/RedHatSatellite/satellite-clone/pull/380 and re-ran automation, it then failed on this:

Liquibase update Failed: liquibase.exception.DatabaseException: org.postgresql.util.PSQLException: The server requested password-based authentication, but no password was provided.
SEVERE 12/23/20 5:01 PM:liquibase: liquibase.exception.DatabaseException: org.postgresql.util.PSQLException: The server requested password-based authentication, but no password was provided.
liquibase.exception.DatabaseException: liquibase.exception.DatabaseException: org.postgresql.util.PSQLException: The server requested password-based authentication, but no password was provided.
        at liquibase.integration.commandline.CommandLineUtils.createDatabaseObject(CommandLineUtils.java:61)
        at liquibase.integration.commandline.Main.doMigration(Main.java:788)
        at liquibase.integration.commandline.Main.main(Main.java:133)
Caused by: liquibase.exception.DatabaseException: org.postgresql.util.PSQLException: The server requested password-based authentication, but no password was provided.
        at liquibase.database.DatabaseFactory.openConnection(DatabaseFactory.java:231)
        at liquibase.database.DatabaseFactory.openDatabase(DatabaseFactory.java:141)
        at liquibase.integration.commandline.CommandLineUtils.createDatabaseObject(CommandLineUtils.java:52)
        ... 2 more
Caused by: org.postgresql.util.PSQLException: The server requested password-based authentication, but no password was provided.
        at org.postgresql.core.v3.ConnectionFactoryImpl.doAuthentication(ConnectionFactoryImpl.java:493)
        at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:205)
        at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:49)
        at org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:195)
        at org.postgresql.Driver.makeConnection(Driver.java:452)
        at org.postgresql.Driver.connect(Driver.java:254)
        at liquibase.database.DatabaseFactory.openConnection(DatabaseFactory.java:223)
        ... 4 more

I'm not sure why candlepin passwords are blank in some of the answers file, but I think that is going to need more discussion after the break and discuss how to deal with it if it is an expected situation.

@anandkumaragrawal for right now, please pull in changes from https://github.com/RedHatSatellite/satellite-clone/pull/380 to here so we can work off only this PR, I have 2 commits in addition to yours. Then we can continue the discussion on potential fixes or perhaps merge and make a follow-up issue to deal with separately.

anandkumaragrawal commented 3 years ago

@johnpmitsch As suggested, I have pulled in the changes from #380 in here. Please review now

johnpmitsch commented 3 years ago

@anandkumaragrawal The update looks good :+1:

I would really like to know the expectations around a blank candlepin password in the answers file before taking any further action with this PR, we'll probably have to wait until everyone's back in the new year to have that discussion.

anandkumaragrawal commented 3 years ago

@johnpmitsch You are absolutely right, @mccun934 has reported the failure due to blank candlepin password twice in the same PR. So, it is really good idea to have round table discussion around it. Lets wait for the folks to retun from New year vacation.

mccun934 commented 3 years ago

FYI: I checked backups from 6.7 and 6.8 for embedded DBs and none had the CP password specified, it was always blank

johnpmitsch commented 3 years ago

Looking at this further:

The installer can use /opt/puppetlabs/puppet/cache/foreman_cache_data/candlepin_db_password for the cp db password when running cpdb update. (see here)

We aren't using this value, rather using the password from the answers file. This is why cpdb update is still failing

This was pre-existing before this PR (it was introduced in remote db support feature), so I'm going to merge this. There will still be a pre-existing bug, this can be tracked in this BZ, but the 6.8 support parts are fine as far as I can tell.