Hi david

Thank you for the review.

The VRS service must be stopped for any of the following configuration
changes and not just changes to vrs_config_file. If you say that
Config-changed would called for other reasons as well the can you
suggest what can be done (Ideally detect stop the service of any of
the parameters in config.yaml had changed, but how to do it?).

And vsc-controller-active,vsc-controller-standby can be either
configured by the user (which vsc is not deployed using charms) or
could be learnt through relation hooks.

Thanks

Subbu

   vsc-controller-active:

    type: string
    description: Active VRS controller to use.
    default:
  vsc-controller-standby:
    type: string
    description: Standby VRS controller to use.
    default:
  vrs-packages:
    type: string
    default: "nuage-metadata-agent nuage-openvswitch-common
nuage-openvswitch-datapath-dkms nuage-python-openvswitch
nuage-openvswitch-switch"
    description: List of packages to install for VRS.
  vrs-repository-url:
    type: string
    description: Optional URL to Nuage VRS repository containing
Debian packages.
    default:
  vrs-ppa-key:
    type: string
    description: Optional KEY to Nuage VRS PPA containing Debian packages.
    default:


Regards
Subbu

On Fri, Oct 30, 2015 at 2:26 PM, David Ames <email address hidden>
wrote:

> Hello, Subbu,
>
> Thank you again for this submission. Having the Nuage services in the
> charmstore will be great.
>
> For the VRS charm the big issue I see is that the VRS service is hard
> stopped and restarted on each config-changed hook run. Config-changed
> hooks get run often, sometimes at times you will not expect. For a low
> level network service like VRS we don't want to see it stopped unless it
> is absolutely necessary as it will presumably cause loss of
> connectivity.
>
> So we will want to gate the restart based on configuration changes. Only
> restart if vrs_config_file changes or some other event that requires a
> service restart. You can check the md5 sum of the configuration file(s)
> at the beginning of the config-changed hook and compare at the end to
> decide if a restart is required.
>
> Gate the following:
> service_stop('nuage-openvswitch-switch')
> vrs_full_restart()
> Can be based on changes to vrs_config_file.
>
>
> The charm has settings for vsc-controller-active and
> vsc-controller-standby as config options. Is there are reason VRS can't
> acquire  vsc-controller-active and vsc-controller-standby from the
> relationship with nuage-vsc with the vrs-controller-service relation?
> If there is a reason, please document the requirement that these config
> options be set in the README and in the description field of the
> config.yaml file.
>
>
> As Adam mentioned we need to see some kind of tests. [1] Preferably these
> are functional tests that we can CI validate. Unit tests are required as a
> minimum.
>
> [1] https://juju.ubuntu.com/docs/authors-testing.html
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1420995
>
> Title:
>   Nuage-Canonical charm: nuage-vrs
>
> Status in Juju Charms Collection:
>   Fix Committed
>
> Bug description:
>   Please review
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/1420995/+subscriptions
>