Skip to content

Conversation

itzikb
Copy link

@itzikb itzikb commented Mar 14, 2017

Adding the option to change baseurl and whether to enable
the repository or not (In case there is already a repository with opendaylight)

@itzikb
Copy link
Author

itzikb commented Mar 14, 2017

@dfarrell07 Can you please check my the job failed?

@dfarrell07
Copy link
Owner

Can you please check my the job failed?

Yeah, was just looking at that. Not sure off-hand, digging.

Any chance you can add Beaker/rspec tests and docs for this?

$odl_rest_port = '8080'
$odl_bind_ip = '0.0.0.0'
$rpm_repo = 'opendaylight-5-testing'
$baseurl = "http://cbs.centos.org/repos/nfv7-$rpm_repo/\$basearch/os/"
Copy link
Owner

Choose a reason for hiding this comment

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

This implementation is a nice way of maintaining backwards compatibility

@itzikb
Copy link
Author

itzikb commented Mar 14, 2017

@dfarrell07 Thanks. Just did it but still fails - https://travis-ci.org/dfarrell07/puppet-opendaylight/builds/211112534

$odl_bind_ip = '0.0.0.0'
$rpm_repo = 'opendaylight-5-testing'
$baseurl = "http://cbs.centos.org/repos/nfv7-${rpm_repo}/\$basearch/os/"
$enable_repo = True
Copy link
Owner

@dfarrell07 dfarrell07 Mar 15, 2017

Choose a reason for hiding this comment

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

This is causing your rspec test failures

     Puppet::PreformattedError:
       Evaluation Error: Resource type not found: True at /home/travis/build/dfarrell07/puppet-opendaylight/spec/fixtures/modules/opendaylight/manifests/params.pp:17:18

https://travis-ci.org/dfarrell07/puppet-opendaylight/jobs/211112601#L1134

@dfarrell07
Copy link
Owner

@itzikb #138 seems like it may provide most/all of the behavior you need. Would just disabling repo management be basically what you're looking for? If no, should be minor changes to expose rpm_enable_repo and add rpm_baseurl on top of the refactor.

@itzikb
Copy link
Author

itzikb commented Mar 19, 2017

@dfarrell07 I agree. #138 seems to better solve the issue of disabling the repository.
We also need to to the change as you suggested.

Adding the option to change baseurl and GPG check for the RPM repository
@itzikb
Copy link
Author

itzikb commented Mar 21, 2017

@dfarrell07 , I have trouble with the new opendaylight::repos tests:
https://travis-ci.org/dfarrell07/puppet-opendaylight/builds/213030036

If I understand correctly it's using the class directly. Can you help?

Thanks

Copy link
Owner

@dfarrell07 dfarrell07 left a comment

Choose a reason for hiding this comment

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

The test code is passing :rpm_repo => 'testing' and expecting :baseurl => 'http://cbs.centos.org/repos/nfv7-testing/$basearch/os/' as the result. Your logic expects baseurl to be passed, is only using rpm_repo to set the name of the yumrepo resource (and therefore the .repo file name). Need the two in sync. Also need to include gpgcheck. Also need to update the Beaker test to use these new params. Would also be great to add some conditionals to cover new params in Beaker install tests and the rpm_validations helper.

baseurl => 'http://cbs.centos.org/repos/nfv7-opendaylight-5-testing/$basearch/os/'
}
```
To disable the repository (For example when a repository for OpenDaylight already exists) use:
Copy link
Owner

Choose a reason for hiding this comment

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

"For" should not be capitalized

$odl_rest_port = $::opendaylight::params::odl_rest_port,
$odl_bind_ip = $::opendaylight::params::odl_bind_ip,
$rpm_repo = $::opendaylight::params::rpm_repo,
$baseurl = "http://cbs.centos.org/repos/nfv7-${rpm_repo}/\$basearch/os/",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this will work. rpm_repo is always reading the one inherited from params because the rpm_repo you are overriding in this class hasn't been parsed yet. So it is never using the value you pass to rpm_repo. One option is to just make baseurl a separate param that is inherited, like this:
$baseurl = $::opendaylight::params::baseurl,

params.pp:
$baseurl = "http://cbs.centos.org/repos/nfv7-${rpm_repo}/\$basearch/os/"

Then the user is responsible to override baseurl as well as rpm_repo if they want to change how the repo is created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants