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

Fixes for Satellite 6.3. This include PR #50, #43 #53

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

Conversation

Rocco83
Copy link
Contributor

@Rocco83 Rocco83 commented May 3, 2018

This fix override also PR #52, making it obsolete.

Also, it include some improvement in clean section.
CCV were possibly done before CV, resulting in a error.
Please note that bug http://projects.theforeman.org/issues/23458 may occur.

Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

overall NACK, mostly because it's too many changes in one commit :)

README.md Outdated
@@ -77,7 +91,8 @@ Example configuration for `cvmanager`:
- application1

* `user`: username of a Satellite 6 user to execute the actions with
* `pass`: password of the same user
* `pass`: password of the same user in cleartext
* `encoded_pass`: password of the same user in base64 encryption (generate with 'echo -n "sat_password" | base64')
Copy link
Member

Choose a reason for hiding this comment

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

NACK, base64 is not an encryption and we should not pretend it to be one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is encoded fine as description or do you prefer something like "masquerade_pass"?

Copy link
Member

Choose a reason for hiding this comment

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

no, I prefer not to have this "feature" ;)

Choose a reason for hiding this comment

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

barely_obfuscated_pass ?

@Rocco83
Copy link
Contributor Author

Rocco83 commented May 3, 2018

Willing to discuss every single change :)
I understand that this is a huge one, but this is also because two old PR has not been checked previously :(

@evgeni
Copy link
Member

evgeni commented May 3, 2018

I merged the labels PR, please rebase, drop the base64 changes and we'll re-discuss.

@Rocco83
Copy link
Contributor Author

Rocco83 commented May 3, 2018

Uploaded as requested without encoded_pass option.

@@ -198,14 +222,14 @@ def clean()
end

def checktask(task, last_date)
task_completed_at = Time.xmlschema(task['ended_at']) rescue Time.parse(task['ended_at'])
task_completed_at = Time.xmlschema(task['started_at']) rescue Time.parse(task['started_at'])
Copy link
Member

Choose a reason for hiding this comment

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

Why this change? If we use started_at, we might find tasks that were started before last_date, but did not complete, and thus were not part of the last publish.

Copy link
Contributor Author

@Rocco83 Rocco83 May 3, 2018

Choose a reason for hiding this comment

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

@bandrea83 I do not recall the details. May you please check?
As reference: f3b7a3a

# Check every repo in the CV
cv['repository_ids'].each do |repo_id|
# get repo data
repo = @api.resource(:repositories).call(:show, {:id => repo_id})
# check if the last sync has been ever completed
if repo.has_key?('last_sync') and repo['last_sync'] and repo['last_sync'].has_key?('ended_at') and repo['last_sync']['ended_at']
Copy link
Member

Choose a reason for hiding this comment

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

Same question as above, why changing to started_at

@gearboxscott
Copy link

gearboxscott commented May 3, 2018

I tested it this morning after everyone made changes and committed the pull request, where they changed name back to label, here are the results. NOT good. It didn't update the composite view.

./cvmanager -c TEST_CCV.yaml --wait update --verbose --force
Inspecting RHEL7_CCV
 Checking RHEL7_CV
    [VERBOSE]   Desired version not found, skipping
Inspecting TEST_CCV
 Checking TEST_CV
    [VERBOSE]   Desired version latest found in CCV
    [VERBOSE]   Found 4.0 as the 'latest' version

the YAML is

[root@sat6 katello-cvmanager]# cat TEST_CCV.yaml
---
:settings:
  :user: admin
  :pass: 1hgabfat
  :uri: https://sat6.consulting-services.lan
  :timeout: 300
  :org: 1
  :lifecycle: 2
  :keep: 5
:cv:
  TEST_CV: latest
:ccv:
  TEST_CCV:
    TEST_CV: latest
:publish:
  - TEST_CV
:promote:
  - TEST_CCV

the reason I changed it to name was because label and latest_version in xml wasn't being picked up by the current code. Satellite version 6.3.1. So maybe someone with a 6.2 version can compare it with xml from 6.3.

I forced a publish and got

root@sat6 katello-cvmanager]# ./cvmanager -c TEST_CCV.yaml --wait publish --verbose --force
    [VERBOSE] Checking Content View Default_Organization_View
    [VERBOSE] Checking Content View OSP10LTS_CV
    [VERBOSE] Checking Content View OSP11_CV
    [VERBOSE] Checking Content View OSP12_CV
    [VERBOSE] Checking Content View OSP13LTS_CV
    [VERBOSE] Checking Content View RHEL7_CCV
    [VERBOSE] Checking Content View RHEL7_CV
    [VERBOSE] Checking Content View TEST_CCV
    [VERBOSE] Checking Content View TEST_CV
Inspecting TEST_CV as listed in CSV
 forced publish, even if there were no changes
Publishing TEST_CV
E, [2018-05-03T09:47:38.466017 #5837] ERROR -- : 500 Internal Server Error
/usr/share/gems/gems/rest-client-1.6.7/lib/restclient/abstract_response.rb:48:in `return!': 500 Internal Server Error (RestClient::InternalServerError)
	from /usr/local/share/gems/gems/apipie-bindings-0.2.2/lib/apipie_bindings/api.rb:355:in `block in rest_client_call_block'
	from /usr/share/gems/gems/rest-client-1.6.7/lib/restclient/request.rb:228:in `call'
	from /usr/share/gems/gems/rest-client-1.6.7/lib/restclient/request.rb:228:in `process_result'
	from /usr/share/gems/gems/rest-client-1.6.7/lib/restclient/request.rb:178:in `block in transmit'
	from /usr/share/ruby/net/http.rb:852:in `start'
	from /usr/share/gems/gems/rest-client-1.6.7/lib/restclient/request.rb:172:in `transmit'
	from /usr/share/gems/gems/rest-client-1.6.7/lib/restclient/request.rb:64:in `execute'
	from /usr/share/gems/gems/rest-client-1.6.7/lib/restclient/request.rb:33:in `execute'
	from /usr/share/gems/gems/rest-client-1.6.7/lib/restclient/resource.rb:67:in `post'
	from /usr/local/share/gems/gems/apipie-bindings-0.2.2/lib/apipie_bindings/api.rb:327:in `call_client'
	from /usr/local/share/gems/gems/apipie-bindings-0.2.2/lib/apipie_bindings/api.rb:240:in `http_call'
	from /usr/local/share/gems/gems/apipie-bindings-0.2.2/lib/apipie_bindings/api.rb:190:in `call_action'
	from /usr/local/share/gems/gems/apipie-bindings-0.2.2/lib/apipie_bindings/api.rb:185:in `call'
	from /usr/local/share/gems/gems/apipie-bindings-0.2.2/lib/apipie_bindings/resource.rb:21:in `call'
	from ./cvmanager:464:in `block in publish'
	from ./cvmanager:358:in `each'
	from ./cvmanager:358:in `publish'
	from ./cvmanager:517:in `<main>'
[root@sat6 katello-cvmanager]#

so I can try update again.

@evgeni
Copy link
Member

evgeni commented May 3, 2018

this works here:

[root@satellite01 katello-cvmanager]# cat test.yaml 
---
:settings:
  :user: admin
  :pass: changeme
  :uri: https://localhost
  :timeout: 300
  :org: 1
  :lifecycle: 2
  :keep: 5
  :verify_ssl: false
:ccv:
  client_one:
    RHEL7: latest
:publish:
  - RHEL7

[root@satellite01 katello-cvmanager]# tfm-ruby ./cvmanager --verbose --wait --config test.yaml publish
    [VERBOSE] Checking Content View client_one
    [VERBOSE] Checking Content View Default_Organization_View
    [VERBOSE] Checking Content View RHEL7
Inspecting RHEL7 as listed in CSV
    [VERBOSE] Checking Content View SatTools
[root@satellite01 katello-cvmanager]# tfm-ruby ./cvmanager --verbose --wait --config test.yaml --force publish
    [VERBOSE] Checking Content View client_one
    [VERBOSE] Checking Content View Default_Organization_View
    [VERBOSE] Checking Content View RHEL7
Inspecting RHEL7 as listed in CSV
 forced publish, even if there were no changes
Publishing RHEL7
    [VERBOSE] Checking Content View SatTools
waiting 10 for pending tasks: ["68dafcaf-d10c-46f6-9afc-11e0f1f3ae0c"]
waiting 20 for pending tasks: ["68dafcaf-d10c-46f6-9afc-11e0f1f3ae0c"]
waiting 30 for pending tasks: ["68dafcaf-d10c-46f6-9afc-11e0f1f3ae0c"]
waiting 40 for pending tasks: ["68dafcaf-d10c-46f6-9afc-11e0f1f3ae0c"]
waiting 50 for pending tasks: ["68dafcaf-d10c-46f6-9afc-11e0f1f3ae0c"]
waiting 60 for pending tasks: ["68dafcaf-d10c-46f6-9afc-11e0f1f3ae0c"]
Silencing output until there's a task status change...

@evgeni
Copy link
Member

evgeni commented May 3, 2018

Can you provide your production.log when the error happened?

@@ -334,10 +397,18 @@ def promote()
if not @options[:noop]
req = @api.resource(:content_view_versions).call(:promote, {:id => latest_version['id'], :environment_id => @options[:lifecycle], :force => @options[:force]})
tasks << req['id']
wait([req['id']]) if @options[:sequential]
#if @options[:sequential] > 0 and tasks.length >= @options[:sequential] # Why are not we using this standard code?
if @options[:sequential] > 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evgeni do you know / recall why here a different code is used?
if @options[:sequential] > 0 in place of if @options[:sequential] > 0 and tasks.length >= @options[:sequential]

@Rocco83
Copy link
Contributor Author

Rocco83 commented May 3, 2018

@gearboxscott thanks for taking the time to test it.
I am preparing also a PR to always rescue 500 errors over "write" call. can be done of course also for index and show, but it will take more time. @evgeni you to tell if i should push it now or not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants