search

att-comdev / openstack-helm

PROJECT HAS MOVED TO OPENSTACK
https://github.com/openstack/openstack-helm
69 stars 41 forks source link

Enable nodeport on/off for services #261

Closed larryrensing closed 7 years ago

larryrensing commented 7 years ago

What is the purpose of this pull request?: Provide boolean to enable/disable nodeports for services What issue does this pull request address?: Fixes #257

Notes for reviewers to consider: Heat values.yaml was formatted differently than the others, did a small modification to keep a consistent naming scheme for ports.

Specific reviewers for pull request: @v1k0d3n @wilkers-steve @alanmeadows @intlabs

v1k0d3n commented 7 years ago

@larryrensing i merged in the service labels for network earlier today, and that caused your merge conflicts. simple changes, but sorry for the re-work. at least it's small/limited.

i agree with @alanmeadows' approach here; per service/network declaration. the example is great, and this provides greater flexibility for operators.

it may be a bit late for this feedback as it pertains to GitHub...but something that could be useful going forward even in gerrit is to pick a single service-level chart as an example, keep the work small, ask for feedback, and then expand on the concepts once you receive initial feedback on the PR or patchset. this will keep the work laser-focused/targeted, easier for initial review of the introduced concepts, reduce your required rework, open up the feedback loop for input earlier (before you've invested a large chunk of time), and then the re-work can be applied as a more sweeping follow-up change in the future.

i just a thought that could help you, @wilkers-steve or anyone else as they're developing and getting initial feedback from @alanmeadows or other community members on conceptual or architectural changes. this is something that i'd like to see not just from our team going forward, but it's a practice i'd like to see from the general community as well. i think it will help limit the introduction of large or disruptive architectural shifts in strategy. as an example, look at @alanmeadows targeted architectural changes/concepts for configuration (#262). even though it's a large concept, the work was laser-focused on keystone to allow feedback before a march larger investment of time was dedicated to the changing concept. i think this approach will help us all going forward.

larryrensing commented 7 years ago

thanks for the feedback!

closing this out, we can figure out a node port structure for a values tree for a single service in a different pull request