Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added eksengine to engines/__init__.py #315

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Added eksengine to engines/__init__.py #315

wants to merge 7 commits into from

Conversation

mikezuniga
Copy link

@mikezuniga mikezuniga commented Jun 6, 2018

  • Added boto3 to setup.py requirements
  • Added eks engine
  • Added eks kubeconfig template
  • Docs added
  • TODO added

current issue related with kqueen-k8s-api methods:

api_1           | HTTP response headers: HTTPHeaderDict({'Content-Type': 'application/json', 'X-Content-Type-Options': 'nosniff', 'Date': 'Sun, 10 Jun 2018 10:12:10 GMT', 'Content-Length': '234'})
api_1           | HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"services is forbidden: User \"system:anonymous\" cannot list services at the cluster scope","reason":"Forbidden","details":{"kind":"services"},"code":403}
api_1           |
api_1           | Traceback (most recent call last):
api_1           |   File "/code/kqueen/models.py", line 124, in status
api_1           |     'addons': kubernetes.list_services(filter_addons=True),
api_1           |   File "/code/kqueen/kubeapi.py", line 238, in list_services
api_1           |     include_uninitialized=include_uninitialized
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/apis/core_v1_api.py", line 14358, in list_service_for_all_namespaces
api_1           |     (data) = self.list_service_for_all_namespaces_with_http_info(**kwargs)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/apis/core_v1_api.py", line 14455, in list_service_for_all_namespaces_with_http_info
api_1           |     collection_formats=collection_formats)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 321, in call_api
api_1           |     _return_http_data_only, collection_formats, _preload_content, _request_timeout)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 155, in __call_api
api_1           |     _request_timeout=_request_timeout)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 342, in request
api_1           |     headers=headers)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/rest.py", line 231, in GET
api_1           |     query_params=query_params)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/rest.py", line 222, in request
api_1           |     raise ApiException(http_resp=r)
api_1           | kubernetes.client.rest.ApiException: (403)

faced when try to parse k8s config body

Added boto3 to setup.py requirements
Added eks engine
Added eks kubeconfig template
'ACTIVE': config.get('CLUSTER_OK_STATE'),
'DELETING': config.get('CLUSTER_DEPROVISIONING_STATE'),
'FAILED': config.get('CLUSTER_ERROR_STATE'),
'UPDATED': config.get('CLUSTER_UPDATING_STATE')

Choose a reason for hiding this comment

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

'UPDATING' ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ekhomyakova I suppose those names comes from EKS, but I don't see updating in the list at all, so may be we can just remove updated from the list

Copy link
Author

Choose a reason for hiding this comment

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

for what i know the engine requires an updating state, so either we leave it or we need to put it to unknown

'number': True
}
},
'roleArn': {

Choose a reason for hiding this comment

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

It's better to use snake_case everywhere

'required': True
}
},
'subnetid': {

Choose a reason for hiding this comment

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

subnet_id

'required': True
}
},
'securitygroupid': {

Choose a reason for hiding this comment

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

security_group_id or securitygroup_id

except Exception as e:
msg = 'Creating cluster {} failed with the following reason:'.format(self.cluster.id)
logger.exception(msg)
return False, msg
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, return only "e" instead of msg, since message will be huge and looks like:

Error occurred with the following reason: Creating cluster failed with the following reason:'

def engine_status(cls, **kwargs):
try:
aws_access_key = kwargs.get('aws_access_key', '')
aws_secret_key = kwargs.get('aws_secret_key', '')
Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean 'aws_secret_access_key'?

'ACTIVE': config.get('CLUSTER_OK_STATE'),
'DELETING': config.get('CLUSTER_DEPROVISIONING_STATE'),
'FAILED': config.get('CLUSTER_ERROR_STATE'),
'UPDATED': config.get('CLUSTER_UPDATING_STATE')
Copy link
Contributor

Choose a reason for hiding this comment

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

@ekhomyakova I suppose those names comes from EKS, but I don't see updating in the list at all, so may be we can just remove updated from the list

"""
Implementation of :func:`~kqueen.engines.base.BaseEngine.provision`
"""
# self.name = kwargs.get('name', 'noname')
Copy link
Contributor

Choose a reason for hiding this comment

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

? :)

return {}
state = STATE_MAP.get(response['cluster']['status'], config.get('CLUSTER_UNKNOWN_STATE'))

key = 'cluster-{}-{}'.format(response['cluster']['name'], response['cluster']['name'])
Copy link
Contributor

Choose a reason for hiding this comment

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

why names are used two times?

Copy link
Author

Choose a reason for hiding this comment

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

one of them should be the id, but EKS uses the name as id at the moment, the only other option is to use the ARN which is extremely long arn:aws:eks:us-west-2:012345678910:cluster/prod

try:
response = self.client.describe_cluster(self.cluster.metadata['id'])
except Exception as e:
msg = 'Fetching data from backend for cluster {} failed with the following reason:'.format(self.cluster.metadata['heat_cluster_id'])
Copy link
Contributor

Choose a reason for hiding this comment

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

self.cluster.metadata['id']

},
'subnetid': {
'type': 'text',
'label': 'Subnet Id',
Copy link
Contributor

Choose a reason for hiding this comment

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

Subnet/s Id - since more than one can be chosen. Ideally we need to get list of them and provide ability to pick subnets on the fly, but for now we need to parse string to the list at least (on the UI part)

aws_secret_access_key=aws_secret_key
)
try:
list(client.list_clusters())
Copy link
Contributor

Choose a reason for hiding this comment

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

can we used more lightweight request here? (since this request is sending for each cluster, even for list all clusters operation)

  1. at least add maxResults parameter=1
  2. or use another request maybe generate_presigned_url?

Copy link
Contributor

Choose a reason for hiding this comment

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

seems, No , coz afaik only list_clusters can provide valid check on all required User IAM roles/policies and etc.. I understand that its huge request, but we are locked by small api

naumvd95 and others added 4 commits June 10, 2018 11:35
- Template structure defined
- Kubeconfig download fixed
- Cluster states fixed
- Engine status fixed
- TODO added

current issue related with kqueen-k8s-api methods:
```
api_1           | Traceback (most recent call last):
api_1           |   File "/code/kqueen/models.py", line 124, in status
api_1           |     'addons': kubernetes.list_services(filter_addons=True),
api_1           |   File "/code/kqueen/kubeapi.py", line 238, in list_services
api_1           |     include_uninitialized=include_uninitialized
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/apis/core_v1_api.py", line 14358, in list_service_for_all_namespaces
api_1           |     (data) = self.list_service_for_all_namespaces_with_http_info(**kwargs)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/apis/core_v1_api.py", line 14455, in list_service_for_all_namespaces_with_http_info
api_1           |     collection_formats=collection_formats)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 321, in call_api
api_1           |     _return_http_data_only, collection_formats, _preload_content, _request_timeout)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 155, in __call_api
api_1           |     _request_timeout=_request_timeout)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 342, in request
api_1           |     headers=headers)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/rest.py", line 231, in GET
api_1           |     query_params=query_params)
api_1           |   File "/usr/local/lib/python3.6/site-packages/kubernetes/client/rest.py", line 222, in request
api_1           |     raise ApiException(http_resp=r)
api_1           | kubernetes.client.rest.ApiException: (403)
```
faced when try to parse k8s config body
- Typo fixes in EKS engine
Copy link
Contributor

@katyafervent katyafervent left a comment

Choose a reason for hiding this comment

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

Please, add new pipfile.lock

},
'cluster': {
'node_count': {
# TODO unsupported, need to define template k8s input
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to comment node_count then

- "-i"
- "<cluster-name>"
# - "-r"
# - "<role-arn>"
Copy link
Contributor

Choose a reason for hiding this comment

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

is it suppose to be commented?

Copy link
Contributor

Choose a reason for hiding this comment

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

for now yes

@naumvd95 naumvd95 added the TBD label Jul 2, 2018
@naumvd95
Copy link
Contributor

naumvd95 commented Jul 2, 2018

related issue kubernetes-client/python#514

Copy link
Contributor

@naumvd95 naumvd95 left a comment

Choose a reason for hiding this comment

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

lets say I'm ok with that, but in fact I just want to hide this PR from my notifications tab :)

Copy link
Contributor

@naumvd95 naumvd95 left a comment

Choose a reason for hiding this comment

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

lets say I'm ok with that, but in fact I just want to hide this PR from my notifications tab :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants