Skip to content

Cleanup sysconfig values as everything is now in a config file#1935

Merged
openshift-bot merged 1 commit intoopenshift:masterfrom
sdodson:only-use-nodeconfig
Apr 29, 2015
Merged

Cleanup sysconfig values as everything is now in a config file#1935
openshift-bot merged 1 commit intoopenshift:masterfrom
sdodson:only-use-nodeconfig

Conversation

@sdodson
Copy link
Member

@sdodson sdodson commented Apr 27, 2015

  • Cleanup several unused items now that everything is stored in config
    files, now the only things in our sysconfig files are the paths to
    the openshift config files and items that should be specified as OPTIONS and cannot be defined in
    the config file.
  • Create /etc/openshift/{master,node} in their respective packages

@sdodson
Copy link
Member Author

sdodson commented Apr 27, 2015

This is effectively re-opening of #1616 as it appears I cannot re-open PRs. Plenty of discussion in that PR but I think this is complementary to #1737

@sdodson sdodson force-pushed the only-use-nodeconfig branch from 8553a6b to 410cd43 Compare April 29, 2015 18:49
* Cleanup several unused items now that everything is stored in config
  files, now the only things in our sysconfig files are the paths to
  the openshift config files and OPTIONS
* Create /etc/openshift/{master,node} in their respective packages
@sdodson sdodson force-pushed the only-use-nodeconfig branch from 410cd43 to b5ffc45 Compare April 29, 2015 18:52
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not certain what to do about this, if they have a config file as the services expect them to then passing this as a commandline argument is ignored, but then we have no way other than generating a config file to set the default registry.

If we care, perhaps this could be adjusted to patch pkg/cmd/util/variable/imagetemplate.go or override it at build time with an ldflag somehow?

Copy link
Member

Choose a reason for hiding this comment

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

With the other changes, the services will choke without a config file anyway. I guess I don't really see a problem with that, although maybe we should supply config files with some workable defaults so that the services will start out of the box? Given we expect to be using the ansible installer, and the rate of change of config, not sure we want that yet though.

I guess LGTM as-is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, sounds good to me.

@sdodson
Copy link
Member Author

sdodson commented Apr 29, 2015

@sosiouxme Mind taking a look at this? It's re-incarnation of #1616 which we tried to merge but kept hitting flakiness and it then ran into merge conflicts and was ultimately closed.

@sosiouxme
Copy link
Member

How about we [merge] then while we can...

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1729/) (Image: devenv-fedora_1397)

@openshift-bot
Copy link
Contributor

Evaluated for origin up to b5ffc45

openshift-bot pushed a commit that referenced this pull request Apr 29, 2015
@openshift-bot openshift-bot merged commit 51099a8 into openshift:master Apr 29, 2015
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