Ori Liel has posted comments on this change.
Change subject: API: host-deploy: Adding ssh username, port and fingerprint to
host information
......................................................................
Patch Set 11: (9 inline comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostResource.java
Line 97:
Line 98: @Override
Line 99: public Response install(Action action) {
Line 100: // REVISIT fencing options
Line 101: VDS vds = getEntity();
call to validateEnums missing
Line 102: UpdateVdsActionParameters params = new
UpdateVdsActionParameters(vds.getStaticData(), action.getRootPassword(), true);
Line 103: if (action.isSetSsh()) {
Line 104: if (action.getSsh().isSetUser()) {
Line 105: if (action.getSsh().getUser().isSetPassword()) {
Line 110: //
params.getvds().setSshUsername(action.getSsh().getUser().getUserName());
Line 111: //}
Line 112: }
Line 113: if (action.getSsh().isSetPort()) {
Line 114:
params.getvds().setSshPort(action.getSsh().getPort().intValue());
no need for .intValue() - action.getSsh().getPort() is an integer.
Line 115: }
Line 116: if (action.getSsh().isSetFingerprint()) {
Line 117:
params.getvds().setSshKeyFingerprint(action.getSsh().getFingerprint());
Line 118: }
Line 122:
map(action.getSsh().getAuthenticationMethod(), null);
Line 123: if (m != null) {
Line 124: params.setAuthMethod(
Line 125: getMapper(AuthenticationMethod.class,
Line 126:
org.ovirt.engine.core.common.action.VdsOperationActionParameters.AuthenticationMethod.class).map(m,
null));
See comment in mapper file - better unite mappers for install and approve
Line 127: }
Line 128: }
Line 129: }
Line 130: if (vds.getVdsType()==VDSType.oVirtNode) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/FeaturesHelper.java
Line 65:
Line 66: private void addSshAuthenticationFeature(Features features) {
Line 67: Feature feature = new Feature();
Line 68: feature.setName("SSH Authentication Method");
Line 69: feature.setDescription("Ability to authenticate by SSH to host
using previlige user password or SSH public key");
previlige --> privileged
Line 70: features.getFeature().add(feature);
Line 71: }
Line 72:
Line 73: private void addWatchdogFeature(Features features) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/HostValidator.java
Line 22: if (host.isSetSsh()) {
Line 23: if (host.getSsh().isSetAuthenticationMethod()) {
Line 24: validateEnum(AuthenticationMethod.class,
host.getSsh().getAuthenticationMethod(), true);
Line 25: }
Line 26: }
Even better would be to have an SshValidator class, which knows how to validate
the Ssh entity, and invoke it from both HostValidator and ActionValidator. Not
critical.
Line 27: }
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendHostsResourceTest.java
Line 313: expect(entity.getId()).andReturn(GUIDS[index]).anyTimes();
Line 314: expect(entity.getName()).andReturn(NAMES[index]).anyTimes();
Line 315:
expect(entity.getHostName()).andReturn(ADDRESSES[index]).anyTimes();
Line 316:
expect(entity.getStatus()).andReturn(VDS_STATUS[index]).anyTimes();
Line 317:
expect(entity.getStaticData()).andReturn(vdsStatic).anyTimes();
Looks like you're doing nothing with this mock. If it's purpose is that
entity.getStaticData() won't return null, you can simply do:
expect(entity.getstaticData()).andReturn(new VdsStatic()), and then you won't
have to add an extra parameter (mock for VdsStatic) to this method.
Line 318: if (statistics != null) {
Line 319: setUpStatisticalEntityExpectations(entity, statistics);
Line 320: }
Line 321: return entity;
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java
Line 545: }
Line 546: }
Line 547:
Line 548: @Mapping(from = String.class, to = AuthenticationMethod.class)
Line 549: public static AuthenticationMethod map(String s,
AuthenticationMethod auth) {
For consistency, please remove this and invoke
AuthenticationMethod.fromValue(s) directly
Line 550: return AuthenticationMethod.fromValue(s);
Line 551: }
Line 552:
Line 553: @Mapping(from = Action.class, to = ApproveVdsParameters.class)
Line 549: public static AuthenticationMethod map(String s,
AuthenticationMethod auth) {
Line 550: return AuthenticationMethod.fromValue(s);
Line 551: }
Line 552:
Line 553: @Mapping(from = Action.class, to = ApproveVdsParameters.class)
Inheritance hierarchy in engine:
VdsOperationActionParameters --> InstallVdsParameters --> ApproveVdsParameters
So if you write a mapper:
map(Action action, VdsOperationActionParameters params)
You'll be able to use it both for approve and for install
Line 554: public static ApproveVdsParameters map(Action action,
ApproveVdsParameters params) {
Line 555: params.setPassword(action.getRootPassword());
Line 556: if (action.isSetSsh()) {
Line 557: if (action.getSsh().isSetUser()) {
....................................................
Commit Message
Line 18: <port>22</port>
Line 19: <fingerprint>aa:bb:cc:dd:ee:ff:aa:bb</fingerprint>
Line 20: <authentication_method>PublicKey</authentication_mathod>
Line 21: <user>
Line 22: <user_name>root</user_name>
Please remove <user_name> from the example, because you don't actually support
it yet.
Line 23: <password></password>
Line 24: ...
Line 25: </user>
Line 26: </ssh>
--
To view, visit http://gerrit.ovirt.org/16685
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic18e91f1602af42e7c73acea5d875c54545cb3c2
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
