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
Validate OS-kubespray cluster before provision #334
Conversation
b7669e5
to
44751a7
Compare
If there is an error in user-input data, return 400 instead of 500. Validation is made for: * valid image * valid flavor * valid avaliability zone * valid ip for dns * valid master node value * valid network
And not initial string
44751a7
to
bc57769
Compare
self.meta["master_count"] = mc | ||
self.meta["slave_count"] = self.cluster.metadata["slave_count"] | ||
|
||
self.meta["dns"] = [validate_ip(ip) for ip in |
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 suggest to place it after all is None
checks.
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.
why? I placed it here since it performs faster then checks below, so if error will be here, response will be sent faster
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.
Why is it faster, I don't get it?
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.
since it doesn't send api call to the openstack
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.
Oh, okay
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.
commented, also i suggest to rename PR as
Validate OS-kubespray cluster before provision #334
resources = { | ||
"masters": [], | ||
"slaves": [], | ||
} | ||
network = self.c.create_network(self.stack_name) | ||
subnet = self.c.create_subnet(network, cidr="10.1.0.0/16", | ||
subnet_name=self.stack_name, | ||
dns_nameservers=dns) | ||
dns_nameservers=self.meta['dns']) |
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.
maybe get() ?
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.
Some fields are required and some are not, let me also send 400 if required field is not provided
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.
okok
router = self.c.create_router(name=self.stack_name, | ||
ext_gateway_net_id=ext_net.id) | ||
ext_gateway_net_id=self.meta['ext_net'].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.
maybe get() ?
flavor=flavor, | ||
servers_range=range(self.meta["slave_count"]), | ||
image=self.meta['image'], | ||
flavor=self.meta['flavor'], |
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.
maybe get() ?
No description provided.