Skip to content

Conversation

@voukka
Copy link

@voukka voukka commented Jan 15, 2015

Please see https://issues.apache.org/jira/browse/SPARK-5246

Fix for local hostname problem in VPC

Choose a reason for hiding this comment

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

Lets rename this to something like resolve-hostname.sh or setup-hostname.sh in the top-level directory

@nchammas
Copy link

This PR is related to: apache/spark#4038

Choose a reason for hiding this comment

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

We need to invoke the new setup-hostname script here -- You can assume that the spark-ec2 directory exists, so just adding a line like bash /root/spark-ec2/setup-hostname.sh should be sufficient

Copy link
Author

Choose a reason for hiding this comment

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

I thought about creation of "module" and putting it in the list of modules in spark-ec2.py before "spark" module.

It might be better to do it your way.

Choose a reason for hiding this comment

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

Yeah - modules are more appropriate for new packages or something like that. For things like fixing hostnames we can just put it in the top level directory

Copy link
Author

Choose a reason for hiding this comment

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

I added it to setup-slave.sh as you suggested. Should it also be invoked from setup.sh?

Choose a reason for hiding this comment

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

No - setup-slave.sh is invoked on all machines (including the master node). So this should be enough

@shivaram
Copy link

Thanks @voukka -- I left some inline comments

@voukka
Copy link
Author

voukka commented Jan 15, 2015

@nchammas thank you! I forgot to mention that these two bugs relate to each other :)

Also
"This pull request is my original work and I contribute it under project's open source license."

@shivaram
Copy link

Thanks for the update. I want to try this on a cluster once before merging. Unfortunately I am out traveling today, tomm -- so it might be Saturday by the time I get a chance.

Choose a reason for hiding this comment

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

@voukka Shouldn't this be if [ -z "${VPC_ID}"] ? -n is true if $VPC_ID is not empty and we want to exit if the string is empty ?

Copy link
Author

Choose a reason for hiding this comment

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

that's correct. I pushed fixed version.

Choose a reason for hiding this comment

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

Minor style comment: We use [[ or [ with if statements in all our scripts. Could you change this and line 26 to match that ?

Copy link
Author

Choose a reason for hiding this comment

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

fixed

On 18 Jan 2015, at 07:35, Shivaram Venkataraman notifications@github.com wrote:

In resolve-hostname.sh:

+#
+
+# Are we in VPC?
+MAC=wget -q -O - http://169.254.169.254/latest/meta-data/mac
+VCP_ID=wget -q -O - http://169.254.169.254/latest/meta-data/network/interfaces/macs/${MAC}/vpc-id
+if [ -n "${VCP_ID}" ]; then

  • echo "nothing to do - instance is not in VPC"

  • exit 0
    +fi

+SHORT_HOSTNAME=hostname
+
+PRIVATE_IP=wget -q -O - http://169.254.169.254/latest/meta-data/local-ipv4
+
+# do changes only if short hostname does not resolve
+if ( ! ping -c 1 -q "${SHORT_HOSTNAME}" > /dev/null 2>&1 ); then
Minor style comment: We use [[ or [ with if statements in all our scripts. Could you change this and line 26 to match that ?


Reply to this email directly or view it on GitHub.

@shivaram
Copy link

Thanks @voukka - LGTM. Merging this

shivaram added a commit that referenced this pull request Jan 18, 2015
@shivaram shivaram merged commit 1320a46 into mesos:branch-1.3 Jan 18, 2015
@voukka voukka deleted the SPARK-5246_voukka_resolving_hostname branch January 19, 2015 06:09
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