-
Notifications
You must be signed in to change notification settings - Fork 339
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
Updated Traffic Ops ISO Generation process to work with management interfaces #1198
Conversation
traffic_ops/app/lib/API/Iso.pm
Outdated
} | ||
|
||
sub is_valid { | ||
my $self = shift; | ||
my $params = shift; | ||
|
||
my $rules = { | ||
fields => [qw/osversionDir hostName domainName rootPass dhcp ipAddress ipNetmask ipGateway ip6Address ip6Gateway interfaceName interfaceMtu disk/], | ||
fields => [qw/osversionDir hostName domainName rootPass dhcp ipAddress ipNetmask ipGateway ip6Address ip6Gateway interfaceName interfaceMtu disk mgmtInterface mgmtIpGateway/], |
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 you need to add 2 more things to "fields". probably not a big deal but i think "fields" is supposed to represent ALL params
mgmtIpAddress
mgmtIpNetmask
<div class="form-group" ng-class="{'has-error': hasError(isoForm.disk), 'has-feedback': hasError(isoForm.disk)}"> | ||
<label class="control-label col-md-2 col-sm-2 col-xs-12">Disk for OS Install</label> | ||
<label class="control-label col-md-2 col-sm-2 col-xs-12">Disk for OS Install*</label> | ||
<div class="col-md-10 col-sm-10 col-xs-12"> | ||
<input id="disk" name="disk" type="text" class="form-control" placeholder='Typical values are "sda"' ng-model="iso.disk" ng-maxlength="8" autofocus> |
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.
make sure you add "required" to your input form element if disk is now required, otherwise TP code looks good
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.
Ok, I figured that was just for fields you wanted to validate. Fixed.
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.
ok, i see your commit but make sure you add "required" to your input form element like this:
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.
Those fields aren't required though?
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, disk is though. Took me a while
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
|
||
return $self->success( $response, "Generate ISO was successful." ); |
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 this change is a problem. you can't just return the response hash you have to wrap it in a 200 which the success method does
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.
or wait, nm, i see on line 83 you are doing it
return $self->success( $response, "Generate ISO was successful." );
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.
This is code I moved to a new method. You can see the success getting returned here: https://github.com/dneuman64/incubator-trafficcontrol/blob/cb149066027e4a7f730353f86356c77be86590e4/traffic_ops/app/lib/API/Iso.pm#L82
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Sometimes a server requires that users access on a management network interface instead of through the payload network interface. Traffic Ops did not have the ability to support passing management network interface information to the kickstart scripts, even though the data was in the database. This change makes it so that management network interface information can be provided and sent to the kickstart scripts.