Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

Fixes #20987 - Update installation media in KCH #534

Merged

Conversation

johnpmitsch
Copy link
Contributor

This was removed at some point, but we should keep it around for
upgraded users. (It was removed because we no longer create
install media for new kickstart repos that you sync)

@theforeman-bot
Copy link

Issues: #20987

@@ -263,6 +263,13 @@ unless @foreman_proxy_content
# Incorrect error message is piped to /dev/null, can be removed when http://projects.theforeman.org/issues/18186 is fixed
# For the same reason, we accept exit code 65 here.
hammer_cmd("capsule update --id #{proxy_id} --url https://#{@new_hostname}:9090 --new-name #{@new_hostname} 2> /dev/null", [0, 65])

STDOUT.puts "Updating installation media paths"
installation_media_ids = hammer_cmd("medium list | grep #{old_hostname} | awk '{ print $1 }'").split("\n")
Copy link
Member

Choose a reason for hiding this comment

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

this needs @old_hostname everywhere :)


STDOUT.puts "Updating installation media paths"
installation_media_ids = hammer_cmd("medium list | grep #{old_hostname} | awk '{ print $1 }'").split("\n")
installation_media_paths = hammer_cmd("medium list | grep #{old_hostname} | awk '{ print $5 }'").split("\n")
Copy link
Member

Choose a reason for hiding this comment

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

hammer medium list will shorten the path if it is too long. e.g.:

9  | Default_Organization/Library/Red_Hat_Server/Red_Hat_Enterprise_Linux_7_Server... | http://sat.example.com/pulp/repos/Default_Organization/Library/con...        

If you use that output to update the path, save the shortened path, instead of the real one, breaking it :(

Copy link
Member

Choose a reason for hiding this comment

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

I think something like this might be fixing it:

old_mediums = JSON.parse(hammer_cmd("--output json medium list --search #{@old_hostname}"))
old_mediums.each do |medium|
  new_path = medium['Path'].sub(@old_hostname, @new_hostname)
  hammer_cmd("medium update --id #{medium['Id']} --path #{new_path}")
end

(untested)

Copy link
Member

@evgeni evgeni Sep 21, 2017

Choose a reason for hiding this comment

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

I propose the following (tested) patch on top of this PR:

--- a/katello-packaging/katello/katello-change-hostname
+++ b/katello-packaging/katello/katello-change-hostname
@@ -6,6 +6,7 @@ require "rubygems"
 require 'yaml'
 require 'shellwords'
 require "json"
+require 'uri'
 
 raise 'Must run as root' unless Process.uid == 0
 
@@ -275,10 +276,12 @@ unless @foreman_proxy_content
   hammer_cmd("capsule update --id #{proxy_id} --url https://#{@new_hostname}:9090 --new-name #{@new_hostname} 2> /dev/null", [0, 65])
 
   STDOUT.puts "Updating installation media paths"
-  installation_media_ids = hammer_cmd("medium list | grep #{@old_hostname} | awk '{ print $1 }'").split("\n")
-  installation_media_paths =  hammer_cmd("medium list | grep #{@old_hostname} | awk '{ print $5 }'").split("\n")
-  installation_media_ids.zip(installation_media_paths).each do |id, path|
-    hammer_cmd("medium update --id #{id} --path #{path.sub! @old_hostname, @new_hostname}")
+  old_media = JSON.parse(hammer_cmd("--output json medium list --search #{@old_hostname}"))
+  old_media.each do |medium|
+    new_path = URI.parse(medium['Path'])
+    new_path.host = @new_hostname
+    new_path = new_path.to_s
+    hammer_cmd("medium update --id #{medium['Id']} --path #{new_path}")
   end
 end

@johnpmitsch johnpmitsch force-pushed the 20987_add_installation_media_paths branch from d3709fe to 5e08a2c Compare September 21, 2017 15:15
This was removed at some point, but we should keep it around for
upgraded users. (It was removed because  we no longer create
install media for new kickstart repos that you sync)
@johnpmitsch johnpmitsch force-pushed the 20987_add_installation_media_paths branch from 5e08a2c to b89d699 Compare September 21, 2017 15:19
@johnpmitsch
Copy link
Contributor Author

Before hnc

[root@change-hostname-34 ~]# hammer -u admin -p changeme medium list
---|----------------------|---------------------------------------------------------------------------------
ID | NAME                 | PATH                                                                            
---|----------------------|---------------------------------------------------------------------------------
1  | CentOS mirror        | http://mirror.centos.org/centos/$version/os/$arch                               
8  | CoreOS mirror        | http://$release.release.core-os.net                                             
2  | Debian mirror        | http://ftp.debian.org/debian                                                    
4  | Fedora Atomic mirror | http://dl.fedoraproject.org/pub/alt/atomic/stable/Cloud_Atomic/$arch/os/        
3  | Fedora mirror        | http://dl.fedoraproject.org/pub/fedora/linux/releases/$major/Server/$arch/os/   
5  | FreeBSD mirror       | http://ftp.freebsd.org/pub/FreeBSD/releases/$arch/$version-RELEASE/             
10 | local_kickstart      | http://change-hostname-34.example.com/pulp/repos/Default_Organization/Library...
9  | long fake mirror     | http://change-hostname-34.example.com/pulp/repos/Default_Organization/Library...
6  | OpenSUSE mirror      | http://download.opensuse.org/distribution/leap/$version/repo/oss                
7  | Ubuntu mirror        | http://archive.ubuntu.com/ubuntu                                                
---|----------------------|---------------------------------------------------------------------------------

hostname change w/o error and then the capsules were updated:

[root@change-hostname-34 ~]# hammer -u admin -p changeme medium list
---|----------------------|---------------------------------------------------------------------------------
ID | NAME                 | PATH                                                                            
---|----------------------|---------------------------------------------------------------------------------
1  | CentOS mirror        | http://mirror.centos.org/centos/$version/os/$arch                               
8  | CoreOS mirror        | http://$release.release.core-os.net                                             
2  | Debian mirror        | http://ftp.debian.org/debian                                                    
4  | Fedora Atomic mirror | http://dl.fedoraproject.org/pub/alt/atomic/stable/Cloud_Atomic/$arch/os/        
3  | Fedora mirror        | http://dl.fedoraproject.org/pub/fedora/linux/releases/$major/Server/$arch/os/   
5  | FreeBSD mirror       | http://ftp.freebsd.org/pub/FreeBSD/releases/$arch/$version-RELEASE/             
10 | local_kickstart      | http://chagned34.example.com/pulp/repos/Default_Organization/Library/content/...
9  | long fake mirror     | http://chagned34.example.com/pulp/repos/Default_Organization/Library/content/...
6  | OpenSUSE mirror      | http://download.opensuse.org/distribution/leap/$version/repo/oss                
7  | Ubuntu mirror        | http://archive.ubuntu.com/ubuntu                                                
---|----------------------|---------------------------------------------------------------------------------

@evgeni
Copy link
Member

evgeni commented Sep 21, 2017

c'est si ne pas un ACK

I would ACK that, but half of the code is mine, so it's probably not a soo good idea ;)

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

LGTM

@johnpmitsch johnpmitsch merged commit f1418fc into Katello:master Sep 21, 2017
@johnpmitsch johnpmitsch deleted the 20987_add_installation_media_paths branch September 21, 2017 15:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants