Use OpenShift API to control the Ansible container #15492
Conversation
lib/container_orchestrator.rb
Outdated
| @@ -0,0 +1,32 @@ | |||
| require 'kubeclient' | |||
There was a problem hiding this comment.
Can we push this down in to the connection method to delay load it?
lib/container_orchestrator.rb
Outdated
| end | ||
|
|
||
| def client_auth_options | ||
| { :bearer_token_file => TOKEN_FILE } |
There was a problem hiding this comment.
Feels like we don't need a method for this considering that the ssl_options don't have a method of their own.
spec/lib/embedded_ansible_spec.rb
Outdated
| end | ||
| end | ||
|
|
||
|
|
|
Looks good @carbonin ! A few minor comments. |
lib/container_orchestrator.rb
Outdated
| URI::HTTPS.build( | ||
| :host => ENV["KUBERNETES_SERVICE_HOST"], | ||
| :port => ENV["KUBERNETES_SERVICE_PORT"], | ||
| :path => "/oapi" |
There was a problem hiding this comment.
Kill the bonus whitespace here between the keys and => to stop rubocop.
carbonin
force-pushed
the
use_openshift_api_to_start_ansible_container
branch
from
6a419cc
to
59b25e2
Compare
carbonin
force-pushed
the
use_openshift_api_to_start_ansible_container
branch
from
59b25e2
to
44868cd
Compare
Gemfile
Outdated
| @@ -55,6 +55,7 @@ gem "hamlit", "~>2.7.0" | |||
| gem "htauth", "2.0.0", :require => false | |||
| gem "inifile", "~>3.0", :require => false | |||
| gem "jbuilder", "~>2.5.0" # For the REST API | |||
| gem "kubeclient", "~>2.4.0", :require => false # For scaling pods at runtime | |||
There was a problem hiding this comment.
Should we be this restrictive on this dependency? Maybe "~>2.4" would be better to avoid conflict with the provider repo? (Assuming they follow semver)
|
This pull request is not mergeable. Please rebase and repush. |
carbonin
force-pushed
the
use_openshift_api_to_start_ansible_container
branch
from
44868cd
to
da932e3
Compare
| @@ -0,0 +1,29 @@ | |||
|
|
|||
There was a problem hiding this comment.
Kill this blank line please. (You vim users and your leading blank lines 
)
lib/embedded_ansible.rb
Outdated
| if MiqEnvironment::Command.is_container? | ||
| container_stop | ||
| else | ||
| services.each { |service| LinuxAdmin::Service.new(service).stop } |
There was a problem hiding this comment.
I'm wondering if this should be extracted to a method like the container stuff is extracted to a method. Then this method just becomes a simple redirection method with no logic.
def self.stop
MiqEnvironment::Command.is_container? ? container_stop : services_stop
endUpon reading that maybe the methods should be stop_containers and stop_services, so that all 3 stop methods "group" themselves together
There was a problem hiding this comment.
If I went with this rename I would probably go with appliance_stop and appliance_disable to match appliance_start which does more than start services in some cases. What do you think?
It's all private interface anyway so we could always change in a follow up.
There was a problem hiding this comment.
Maybe application_enable and application_disable
…t API As a first pass it implements a #scale method which can be used to change the desired number of replicas in a deployment config. It also handles authenticating to the API using the pod's service account token.
We require this directly now that we are using it from the core application to scale pods at runtime
carbonin
force-pushed
the
use_openshift_api_to_start_ansible_container
branch
from
da932e3
to
463922e
Compare
|
Checked commits carbonin/manageiq@3774758~...02ef655 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
| Kubeclient::Client.new( | ||
| manager_uri, | ||
| :auth_options => { :bearer_token_file => TOKEN_FILE }, | ||
| :ssl_options => { :verify_ssl => OpenSSL::SSL::VERIFY_NONE } |
There was a problem hiding this comment.
friendly reminder: will need something better than VERIFY_NONE eventually :)
This is to be used when MIQ runs inside the cluster, right?
https://kubernetes.io/docs/tasks/tls/managing-tls-in-a-cluster/#trusting-tls-in-a-cluster
sounds like we can use /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
This PR mainly adds a new class called
ContainerOrchestrator.Its purpose is to abstract calls to the OpenShift API into easily consumable methods like
#scalewhich will scale a given deployment config to a particular number of replicas.In this PR I also change the
EmbeddedAnsibleclass to utilize this in order to scale the ansible pod up when we start the embedded ansible role and scale it down when the role is disabled.This ability requires some additional permissions for the server container which is handled using the server's service account in a separate PR to the manageiq-pods repo (ManageIQ/manageiq-pods#174)