Skip to content
This repository has been archived by the owner on Dec 8, 2020. It is now read-only.

merge changes for 6.3 in master. #52

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

merge changes for 6.3 in master. #52

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
06812f6
Updated to work with Satellite 6.3 which is now automatically updatin…
gearboxscott Apr 27, 2018
e298e14
added comments about using cvmanager with Satellite 6.3.
gearboxscott Apr 27, 2018
375c7a4
add comment about this version will not work with 6.2 or less.
gearboxscott Apr 27, 2018
54d2a87
add origin to show where the original code came from.
gearboxscott Apr 27, 2018
361bd77
re-uploaded the file
gearboxscott Apr 27, 2018
7572c36
changed ruby to tfm-ruby
gearboxscott May 1, 2018
7576a29
removed tfm from ruby
gearboxscott May 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
# Satellite 6.3 Update

Satellite 6.3 no longer provide apipie via the repositories. To use this utility, install apipie.
Copy link
Member

Choose a reason for hiding this comment

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

Not completely true, we just don't provide apipie-bindings for the system ruby.

I think we should document the various ways users can run it, and let them decide:

  1. the most Ruby way would be bundle:
bundle install
bundle exec ruby ./cvmanager …
  1. scl enable tfm -- ./cvmanager works on any production install of Katello/Satellite
  2. gem install apipie-bindings followed by a ./cvmanager also works, but is the most unclean one, imho


```
gem install apipie
```

This version will no longer work with Satellite 6.2 or less.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not to make an API call to check if latest is supported or not?
So the code may be backward compatible.

Copy link
Author

Choose a reason for hiding this comment

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

@Rocco83, when @mccun934 told me to merge the changes into the current code for 6.3 and they can download a prior release for 6.2 or less. We can add this functionality but then we need to add in the update section of the code a branch for 6.2 or less. @mccun934 do you me to do that or have them download a prior release?

Copy link
Member

Choose a reason for hiding this comment

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

How big is the diff between 6.2 and 6.3 compat? I'd prefer to remain 6.2 compatibility, but if it's too much of a burden, I'd tag a last 6.2 compatible release and carry on with the modern code.


# Origin

This was forked from katello-cvmanger https://github.com/RedHatSatellite/katello-cvmanager and updated to work with Satellite 6.3.
Copy link
Member

Choose a reason for hiding this comment

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

This is not a fork, it's a tribute, erm, PR ;)


# cvmanager

For automation of some common tasks related to Content Views we created a tool called `cvmanager`. It consists of a Ruby script (`cvmanager`) and a YAML-formatted configuration file (`cvmanager.yaml`). The various features are described in the following chapters.
Expand Down
24 changes: 6 additions & 18 deletions cvmanager
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,11 @@ def update()
# either the version for the component in this CCV is set
# or it is set globally
# never touch non-mentioned components
if @yaml[:ccv].is_a?(Hash) and @yaml[:ccv].has_key?(ccv['label']) and @yaml[:ccv][ccv['label']].has_key?(component['content_view']['label'])
desired_version = @yaml[:ccv][ccv['label']][component['content_view']['label']]
if @yaml[:ccv].is_a?(Hash) and @yaml[:ccv].has_key?(ccv['label']) and @yaml[:ccv][ccv['label']].has_key?(component['content_view']['name'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to rely on name?
This is against #50

Copy link
Author

Choose a reason for hiding this comment

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

@Rocco83 , keep in mind I'm not the originator of the code, my sole purpose of updated was to fix it for 6.3 so my current customer using the code didn't have their periodic based update process for server calling this from cron breaking. I don't agree with the all the logic here because I would like had a method of chaining a publish+update+promote be in one step. I just wanted get it working for 6.3 customers. If we want to go back a visit the logic and rewrite a good portion of then let do that in a different pull request. Or have completely missed the point of fixing this and we need to rewrite some of the flow?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand the point, why is a change needed here?
Sorry but i am not getting that.

Copy link
Author

Choose a reason for hiding this comment

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

@Rocco83 when you run the existing code as it was on a 6.3, the composite content view is marked latest, meaning as you know, you don't have to visit the composite view and update the version of the content views within it. You just have to push. The code, was looking for, something to update and didn't know how to recognize that it's already set to latest and it just needs to be published. the update just run and spews out the version but it doesn't publish, it exits. Try it out, run it on a sat 6.2 and then again on 6.3. 6.2 will change, 6.3 will just exit.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am just referring to this portion of code. Why asking for name instead of label would do the trick?
Am I wrong if i say that what would do the job is in line 274?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, labels should be labels. IMHO. Need to re-run that code with latest katello, tho.

desired_version = @yaml[:ccv][ccv['label']][component['content_view']['name']]
puts_verbose " Desired version #{desired_version} found in CCV"
elsif @yaml[:cv].is_a?(Hash) and @yaml[:cv].has_key?(component['content_view']['label'])
desired_version = @yaml[:cv][component['content_view']['label']]
elsif @yaml[:cv].is_a?(Hash) and @yaml[:cv].has_key?(component['content_view']['name'])
desired_version = @yaml[:cv][component['content_view']['name']]
puts_verbose " Desired version #{desired_version} found in CV"
else
puts_verbose " Desired version not found, skipping"
Expand All @@ -268,22 +268,10 @@ def update()
desired_version = cvversions[0]['version']
puts_verbose " Found #{desired_version} as the 'latest' version"
end

# if the version of the component does not match the one the user requested update it
if component['version'].to_s != desired_version.to_s
puts " Updating from #{component['version']} to #{desired_version}"
oldids = ids.dup
ids.delete(component['id'])
cvversions = @api.resource(:content_view_versions).call(:index, {:content_view_id => component['content_view']['id'], :version => desired_version})
desired_version_id = cvversions['results'][0]['id']
ids.push(desired_version_id)
puts " Old components: #{oldids}"
puts " New components: #{ids}"
# do the update
was_updated = true
end
# end loop
Copy link
Contributor

Choose a reason for hiding this comment

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

Stylistic approach, i think to be agreed with @evgeni

end

was_updated = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why always was_updated = true?

Copy link
Contributor

Choose a reason for hiding this comment

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

The following is taken as example from a CCV API call

content_view_components	
  0	
    latest	false <-- this is true if latest is selected.

Given that, if latest exists and is true, then we can proceed.
Also, I think that the best would be to check anyway if we have the latest version or not of a CV in a CCV.
The reason is, we are going to publish anyway always a new CCV for nothing.

The whole idea of cvmanager is to do actions only if necessary, i think.

My 2 cents,

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, latest should be a special case, and given the API tells us about it, we can handle it properly, instead of removing the code that handles explicit versions.

That's the only 6.3 change, right?

if was_updated
#Change the member content view versions; We do this once at the end, so if there was multiple CV changes, its only one call
puts " Committing new content view versions"
Expand Down