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

Associates the Public Ip after booting up the new instance #152

Merged
merged 2 commits into from
Jun 19, 2017

Conversation

Cesarla
Copy link
Contributor

@Cesarla Cesarla commented Jun 15, 2017

It adds a new pre_configure step that takes care of associating the EIP.

)
ec2.associate_address(InstanceId=instance_id, PublicIp=saved_instance['PublicIpAddress'])
logger.info("Rebooting new instance")
ec2.reboot_instances(InstanceIds=[instance_id])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it still works with create_cluster.py: there we don't reboot the instance. Can you please describe again why is this needed? I remember that Taupage was not able to find the EBS volume unless public IP was associated. Maybe it is a bug in Taupage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me investigate a bit further

Copy link
Collaborator

@a1exsh a1exsh Jun 16, 2017

Choose a reason for hiding this comment

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

I've found that it really fails in taupage-init at step 03-push-taupage-yaml.py:

taupage-init: INFO: Pushing Taupage YAML to <URL>
taupage-init: ERROR: Failed to push Taupage YAML
taupage-init: Traceback (most recent call last):
taupage-init:   File "././init.d/03-push-taupage-yaml.py", line 50, in main
taupage-init:     'Authorization': 'Basic {}'.format(userAndPass)})
taupage-init:   File "/usr/local/lib/python3.4/dist-packages/requests/api.py", line 112, in post
taupage-init:     return request('post', url, data=data, json=json, **kwargs)
taupage-init:   File "/usr/local/lib/python3.4/dist-packages/requests/api.py", line 58, in request
taupage-init:     return session.request(method=method, url=url, **kwargs)
taupage-init:   File "/usr/local/lib/python3.4/dist-packages/requests/sessions.py", line 513, in request
taupage-init:     resp = self.send(prep, **send_kwargs)
taupage-init:   File "/usr/local/lib/python3.4/dist-packages/requests/sessions.py", line 623, in send
taupage-init:     r = adapter.send(request, **kwargs)
taupage-init:   File "/usr/local/lib/python3.4/dist-packages/requests/adapters.py", line 496, in send
taupage-init:     raise ConnectTimeout(e, request=request)
taupage-init: requests.exceptions.ConnectTimeout: HTTPSConnectionPool(host='<HOST>', port=443): Max retries exceeded with url: <PATH> (Caused by ConnectTimeoutError(<urllib3.connection.VerifiedHTTPSConnection object at 0x7f500a3f6710>, 'Connection to <HOST> timed out. (connect timeout=5)'))

What we do in create script is associate the public IP as soon as the instance state changes from 'pending'. But I'm not sure why a reboot is needed: if only to fix it if it has already failed to boot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, it's not needed to reboot the instance.

set_state(ec2, volume, 'created')


def configure_instance(ec2: object, volume: dict, options: dict):
def pre_configure_instance(ec2: object, volume: dict, saved_instance: dict):
if 'PublicIpAddress' in saved_instance:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could check if we need to assign public IP in create_instance() and then set the state to either 'created' or some other state which will be this extra step before going to 'created'.

Not sure what would be the good state name. How about 'public-ip-needed'? Then this step function name would be assign_public_ip(), for example.

@Cesarla Cesarla force-pushed the associate-elastic-ip branch from 89f46f0 to 70e299e Compare June 19, 2017 08:31
@Cesarla
Copy link
Contributor Author

Cesarla commented Jun 19, 2017

@a1exsh I've addressed your comments

@a1exsh
Copy link
Collaborator

a1exsh commented Jun 19, 2017

Wonderful, thank you!

@a1exsh
Copy link
Collaborator

a1exsh commented Jun 19, 2017

👍

@a1exsh
Copy link
Collaborator

a1exsh commented Jun 19, 2017

👍

1 similar comment
@slitsche
Copy link
Collaborator

👍

@slitsche slitsche merged commit 0c46f4f into zalando-stups:master Jun 19, 2017
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.

None yet

3 participants