-
Notifications
You must be signed in to change notification settings - Fork 18
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
Conversation
planb/update_cluster.py
Outdated
) | ||
ec2.associate_address(InstanceId=instance_id, PublicIp=saved_instance['PublicIpAddress']) | ||
logger.info("Rebooting new instance") | ||
ec2.reboot_instances(InstanceIds=[instance_id]) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
planb/update_cluster.py
Outdated
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: |
There was a problem hiding this comment.
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.
89f46f0
to
70e299e
Compare
@a1exsh I've addressed your comments |
Wonderful, thank you! |
👍 |
👍 |
1 similar comment
👍 |
It adds a new pre_configure step that takes care of associating the EIP.