Resolve neutron feedback from PR#60#93
Conversation
This commit addresses: * Separating out stacked ovs daemonset into separate daemonsets. * Fixes line ending issues. * Enhances agents ovs pre-flight checks by using neutron-sanity-check.
intlabs
left a comment
intlabs
left a comment
There was a problem hiding this comment.
Overall looking good, the suggestions I have are not really in the scope of this commit - so merging as it would be ok I think, but may be work addressing sooner rather than later.
| - name: DEPENDENCY_JOBS | ||
| value: "{{ include "joinListWithColon" .Values.dependencies.ovs_agent.jobs }}" | ||
| - name: DEPENDENCY_SERVICE | ||
| value: "{{ include "joinListWithColon" .Values.dependencies.ovs_agent.service }}" |
There was a problem hiding this comment.
We should move this (dependency checks) to an init-container - to both come in line with other services and provide flexibility in the images we can use.
There was a problem hiding this comment.
Agreed, plan is to tackle that as part of #89
| fieldRef: | ||
| fieldPath: metadata.namespace | ||
| - name: COMMAND | ||
| value: "bash /tmp/openvswitch-db-server.sh" |
There was a problem hiding this comment.
As above (move to init-container).
| fieldRef: | ||
| fieldPath: metadata.namespace | ||
| - name: COMMAND | ||
| value: "bash /tmp/openvswitch-vswitchd.sh" |
There was a problem hiding this comment.
As above (move to init container).
| daemonset: | ||
| - neutron-openvswitch | ||
| openvswitchagent: | ||
| ovs_agent: |
There was a problem hiding this comment.
Think this whole section could do with reviewing (though possibly out of scope for this ps). Off the top of my head it could be changed to:
dependencies:
server:
jobs:
- neutron-db-sync
service:
- rabbitmq
- mariadb
- keystone-api
- memcached
dhcp:
service:
- neutron-server
- rabbitmq
- nova-api
daemonset:
- neutron-openvswitch
metadata:
service:
- neutron-server
- nova-api
daemonset:
- neutron-openvswitch
ovs_agent:
service:
- keystone-api
- rabbitmq
- neutron-server
l3:
service:
- neutron-server
- rabbitmq
- nova-api
daemonset:
- neutron-openvswitch
db_sync:
jobs:
- neutron-init
service:
- mariadb
init:
jobs:
- mariadb-seed
service:
- mariadb
post:
jobs:
- neutron-db-sync
service:
- keystone-api
There was a problem hiding this comment.
Sure. Let me examine the diff and get back to you on this.
There was a problem hiding this comment.
This has been addressed. Some of the jobs cannot be removed as I imagine you were collapsing them because of the daemonset dependency which would by its nature ensure they completed. We are not yet leveraging the daemonset dependencies because of #88 but otherwise, all other suggestions applied.
| fieldRef: | ||
| fieldPath: metadata.namespace | ||
| - name: COMMAND | ||
| value: "bash /tmp/openvswitch-vswitchd.sh" |
There was a problem hiding this comment.
As above (change to init container)
| # which means we need to do a create action | ||
| # | ||
| # see https://github.com/att-comdev/openstack-helm/issues/88 | ||
| timeout 3m neutron-sanity-check --config-file /etc/neutron/neutron.conf --ovsdb_native --nokeepalived_ipv6_support |
There was a problem hiding this comment.
This is cool, I was not aware of this command.
There was a problem hiding this comment.
It is quite handy, even with the dependency stuff in place it makes sense at some level to take advantage of it as it can catch errors early and exit. The neutron agent tends to mask errors such as failures working with br-int and just hang or skip foundational setup which is a terrible thing to do because it 'looks' like its working when it will never properly pass packets.
| path: /lib/modules | ||
| - name: run | ||
| hostPath: | ||
| path: /run No newline at end of file |
3 participants
This commit addresses:
Separating out stacked ovs daemonset into separate daemonsets.
Fixes line ending issues.
Enhances agents ovs pre-flight checks by using neutron-sanity-check since we can no longer rely on container dependencies and daemonset dependencies in entrypoint are not yet working.
This change is