Skip to content

[OSJC-101] Add support for setting and retrieving additional gear storag... #148

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[OSJC-101] Add support for setting and retrieving additional gear storag... #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/main/java/com/openshift/client/IApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.jcraft.jsch.JSchException;
import com.jcraft.jsch.Session;
import com.openshift.client.cartridge.ICartridge;
import com.openshift.client.cartridge.IDeployedStandaloneCartridge;
import com.openshift.client.cartridge.IEmbeddableCartridge;
import com.openshift.client.cartridge.IEmbeddedCartridge;
import com.openshift.client.cartridge.IStandaloneCartridge;
Expand Down Expand Up @@ -106,6 +107,13 @@ public interface IApplication extends IOpenShiftResource {
*
*/
public IStandaloneCartridge getCartridge();

/**
* Returns the cartridge (application type) that this app is running on.
*
* @return the deployed standalone cartridge of this application
*/
public IDeployedStandaloneCartridge getStandaloneCartridge();

/**
* Adds the given embeddable cartridge to this application.
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/com/openshift/client/IGearGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,12 @@ public interface IGearGroup {
* @return the gears
*/
public Collection<ICartridge> getCartridges();

/**
* Returns the additional storage configured for this gear group in gigabytes
*
* @return the additional storage value
*/
public int getAdditionalStorage();

}
1 change: 1 addition & 0 deletions src/main/java/com/openshift/client/cartridge/ICartridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public interface ICartridge {
*/
public URL getUrl();


Copy link
Member

Choose a reason for hiding this comment

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

why choose a somehow exotic naming scheme ("update") and not use the common "setXX" naming scheme?

Copy link
Member

Choose a reason for hiding this comment

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

I only see StandaloneCartridgeResource and EmbeddedCartridgeResource to request additional storage, the other cartridge classes simply store the value (EmbeddableCartridge, StandaloneCartridge). IMHO we should then not add #updateAdditionalGearStorage to ICartridge but have this only in specific interfaces (IEmbeddedCartridge and IDeployedStandaloneCartridge (?)) since there's only added value for the deployed variants (StandaloneCartridgeResource and EmbeddedCartridgeResource) and no logic/added value for the cartridge definition classes (StandaloneCartridge, EmbeddableCartridge)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adietish I chose this name explicitly because of the information in the API documentation which is:
'additional_gear_storage Set additional filesystem storage in gigabytes for the gear profile that the cartridge is running on.' I confess I don't remember, if I tested it to see if it is truly a 'set' operation which would make sense given I think you would want to be able to lower or raise it depending upon your needs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adietish In regards to where this interface method should reside, if you believe it should move to a separate interface I could be convinced to change it. Maybe this is an opportunity to define a capability for cartridges. Could it be a 'deployed cartridge'? Is there any significance to identifying it as 'embedded' or 'standalone'? I also havent looked, but does that also require us to cast when we use it?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I dont think that we should introduce major new design elements in the v2 library. I would simply make sure we have this in the 2 interfaces where we need it: the interface for StandaloneCartridgeResource (IDeployedStandaloneCartridge?) and IEmbeddedCartridge

public CartridgeType getType();

/**
Expand Down
27 changes: 27 additions & 0 deletions src/main/java/com/openshift/client/cartridge/IDeployedStandaloneCartridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*******************************************************************************
* Copyright (c) 2014 Red Hat, Inc.
* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v1.0 which accompanies this distribution,
* and is available at http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
******************************************************************************/
package com.openshift.client.cartridge;

/**
* Represents a standalone cartridge that has been deployed as opposed to IStandaloneCartridge
* which really represents the metadata about a standalone cartridge
*/
public interface IDeployedStandaloneCartridge extends IStandaloneCartridge{

/**
* set the additional gear storage for the cartridge to the given
* size.
*
* @param size The total additional gear storage for the cartridge
* in gigabytes
*/
public void setAdditionalGearStorage(int size);
}
19 changes: 9 additions & 10 deletions src/main/java/com/openshift/client/cartridge/IEmbeddedCartridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
******************************************************************************/
package com.openshift.client.cartridge;

import java.net.URL;

import com.openshift.client.IApplication;
import com.openshift.client.IOpenShiftResource;
import com.openshift.client.OpenShiftException;
Expand All @@ -23,14 +21,6 @@
*/
public interface IEmbeddedCartridge extends IOpenShiftResource, IEmbeddableCartridge {

/**
* The url at which this cartridge may be reached
*
* @return the url for this cartridge
* @throws OpenShiftException
*/
public URL getUrl() throws OpenShiftException;

/**
* Destroys this cartridge (and removes it from the list of existing cartridges)
*
Expand All @@ -52,4 +42,13 @@ public interface IEmbeddedCartridge extends IOpenShiftResource, IEmbeddableCartr
* @return the resource properties
*/
public CartridgeResourceProperties getProperties();

/**
* set the additional gear storage for the cartridge to the given
* size.
*
* @param size The total additional gear storage for the cartridge
* in gigabytes
*/
public void setAdditionalGearStorage(int size);
Copy link
Member

Choose a reason for hiding this comment

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

what looks weird to me is that we set the additional gear storage in the standalone cartridge while we can only get it in the gear group. Furthermore it looks to me as if you set the additional gear storage and then go and get the value from the gear group you'd get the old value?
I'm taking care of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The odd ways of setting and getting are in line with what the REST API
provides. Your comment with regards to the value mismatch is probably
correct. We could correct this by either forcing a refresh or figuring
out how to update the gear group when setting the value on the Cartridge.

On 10/14/2014 01:33 PM, André Dietisheim wrote:

In src/main/java/com/openshift/client/cartridge/IEmbeddedCartridge.java:

@@ -52,4 +42,13 @@
* @return the resource properties
*/
public CartridgeResourceProperties getProperties();

  • /**
  • \* set the additional gear storage for the cartridge to the given

  • \* size.

  • *

  • \* @param size  The total additional gear storage for the cartridge

  • \*              in gigabytes

  • */
  • public void setAdditionalGearStorage(int size);

what looks weird to me is that we set the additional gear storage in the
standalone cartridge while we can only get it in the gear group.
Furthermore it looks to me as if you set the additional gear storage and
then go and get the value from the gear group you'd get the old value?

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/openshift-java-client/pull/148/files#r18844358.

Jeff Cantrill
Senior Software Engineer, Red Hat Engineering
Red Hat
Office: 703-748-4420 | 866-546-8970 ext. 8162420
jcantril@redhat.com
http://www.redhat.com

Copy link
Member

Choose a reason for hiding this comment

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

I added a IDeployedStandaloneCartridge#getAdditionalGearStorage and delegated to ApplicationResource behind the scenes: https://github.com/openshift/openshift-java-client/pull/167/files#diff-7f982b00d649ba3eb211bedaee0e4986R68 Since ApplicationResource currently wont cache the gear groups (TODO comment exists) we wont get hurt, any call to it would reload the gear groups. I usually "fix" this by force reloading the outdated resource as you mentioned above. I filed https://issues.jboss.org/browse/OSJC-130 for the missing caching even though it's very unlikely that we'll ever resolve it (moving to v3 client lib ahead)

}
35 changes: 34 additions & 1 deletion src/main/java/com/openshift/client/cartridge/StandaloneCartridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import com.openshift.internal.client.APIResource;
import com.openshift.internal.client.ApplicationResource;
import com.openshift.client.IApplication;
import com.openshift.internal.client.CartridgeType;
import com.openshift.internal.client.cartridge.BaseCartridge;

Expand Down Expand Up @@ -60,4 +61,36 @@ public StandaloneCartridge(String name, URL url, String displayName, String desc
public CartridgeType getType() {
return CartridgeType.STANDALONE;
}
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof IStandaloneCartridge)) {
return false;
}
IStandaloneCartridge other = (IStandaloneCartridge) obj;
// shortcut: downloadable cartridges get their name only when
// they're deployed thus should equal on url only
if (isDownloadable()) {
if (other.isDownloadable()) {
if (getUrl() == null) {
return other.getUrl() == null;
}
return getUrl().equals(other.getUrl());
}
}
if (getName() == null) {
if (other.getName() != null) {
return false;
}
} else if (!getName().equals(other.getName())) {
return false;
}
return true;
}
}
19 changes: 10 additions & 9 deletions src/main/java/com/openshift/internal/client/ApplicationResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import com.openshift.client.OpenShiftException;
import com.openshift.client.OpenShiftSSHOperationException;
import com.openshift.client.cartridge.ICartridge;
import com.openshift.client.cartridge.IDeployedStandaloneCartridge;
import com.openshift.client.cartridge.IEmbeddableCartridge;
import com.openshift.client.cartridge.IEmbeddedCartridge;
import com.openshift.client.cartridge.IStandaloneCartridge;
Expand Down Expand Up @@ -107,7 +108,7 @@ public class ApplicationResource extends AbstractOpenShiftResource implements IA
private Date creationTime;

/** The cartridge (application type/framework) of this application. */
private IStandaloneCartridge cartridge;
private IDeployedStandaloneCartridge cartridge;

/** The scalability enablement. */
private ApplicationScale scale;
Expand Down Expand Up @@ -239,6 +240,11 @@ public IStandaloneCartridge getCartridge() {
return cartridge;
}

@Override
public IDeployedStandaloneCartridge getStandaloneCartridge() {
return cartridge;
}

/**
* Returns the main (standalone) cartrige or one of the embedded cartridges
* whose name matches the given param.
Expand Down Expand Up @@ -449,7 +455,7 @@ private void updateCartridges(Map<String, CartridgeResourceDTO> cartridgeDTOByNa
for (CartridgeResourceDTO cartridgeDTO : cartridgeDTOByName.values()) {
switch(cartridgeDTO.getType()) {
case STANDALONE:
createStandaloneCartrdige(cartridgeDTO);
createStandaloneCartridge(cartridgeDTO);
break;
case EMBEDDED:
addOrUpdateEmbeddedCartridge(cartridgeDTO.getName(), cartridgeDTO);
Expand All @@ -460,13 +466,8 @@ private void updateCartridges(Map<String, CartridgeResourceDTO> cartridgeDTOByNa
}
}

private void createStandaloneCartrdige(CartridgeResourceDTO cartridgeDTO) {
this.cartridge = new StandaloneCartridge(
cartridgeDTO.getName(),
cartridgeDTO.getUrl(),
cartridgeDTO.getDisplayName(),
cartridgeDTO.getDescription(),
cartridgeDTO.getObsolete());
private void createStandaloneCartridge(CartridgeResourceDTO dto) {
this.cartridge = new StandaloneCartridgeResource(dto,this);
}

private void addOrUpdateEmbeddedCartridge(String name, CartridgeResourceDTO cartridgeDTO) {
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/com/openshift/internal/client/EmbeddedCartridgeResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
import com.openshift.client.cartridge.EmbeddableCartridge;
import com.openshift.client.cartridge.IEmbeddableCartridge;
import com.openshift.client.cartridge.IEmbeddedCartridge;
import com.openshift.internal.client.httpclient.request.Parameter;
import com.openshift.internal.client.response.CartridgeResourceDTO;
import com.openshift.internal.client.response.CartridgeResourceProperties;
import com.openshift.internal.client.utils.IOpenShiftJsonConstants;

/**
* A cartridge that is embedded into an application.
Expand All @@ -28,6 +30,7 @@
public class EmbeddedCartridgeResource extends AbstractOpenShiftResource implements IEmbeddedCartridge {

private static final String LINK_DELETE_CARTRIDGE = "DELETE";
private static final String LINK_UPDATE_CARTRIDGE = "UPDATE";

private final String name;
private String displayName;
Expand Down Expand Up @@ -98,6 +101,16 @@ protected void update(CartridgeResourceDTO dto) {
setLinks(dto.getLinks());
}

/**
* Update the additional gear storage for this cartridge to the given value
*
* @param size The total size of storage in gigabytes for the gear
*/
@Override
public void setAdditionalGearStorage(int size) {
new UpdateCartridgeRequest().execute(new Parameter(IOpenShiftJsonConstants.PROPERTY_ADDITIONAL_GEAR_STORAGE, String.valueOf(size)));

}
/**
* Refreshes the content of this embedded cartridge. Causes all embedded
* cartridges of the same application to get updated.
Expand Down Expand Up @@ -170,4 +183,9 @@ private DeleteCartridgeRequest() {
super(LINK_DELETE_CARTRIDGE);
}
}
private class UpdateCartridgeRequest extends ServiceRequest {
private UpdateCartridgeRequest() {
super(LINK_UPDATE_CARTRIDGE);
}
}
}
15 changes: 12 additions & 3 deletions src/main/java/com/openshift/internal/client/GearGroupResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public class GearGroupResource extends AbstractOpenShiftResource implements IGea
/** the gears of this gear group resource */
private final Collection<IGear> gears;

/** the additional storage configured for this gear group, in gigabytes */
private final int additionalStorage;

/** the cartridges in this gear group resource */
private final Collection<ICartridge> cartridges;

Expand All @@ -52,15 +55,17 @@ public class GearGroupResource extends AbstractOpenShiftResource implements IGea
* @param name the gear group's name
* @param gearDTOs the gear group's gears
* @param cartridgeDTOs the gear group's cartridges, indexed by their name
* @param additionalStorage the gear group's additional storage value
* @param application the gear group's parent application
* @param service the underlying REST Service
*/
protected GearGroupResource(final String uuid, final String name, final Collection<GearResourceDTO> gearDTOs,
final Map<String, CartridgeResourceDTO> cartridgeDTOs, final ApplicationResource application, final IRestService service) {
final Map<String, CartridgeResourceDTO> cartridgeDTOs, final int additionalStorage, final ApplicationResource application, final IRestService service) {
super(service);
this.uuid = uuid;
this.name = name;
this.gears = new ArrayList<IGear>();
this.additionalStorage = additionalStorage;
for(GearResourceDTO dto : gearDTOs) {
this.gears.add(new Gear(dto.getUuid(), new GearState(dto.getState()), dto.getSshUrl()));
}
Expand All @@ -82,7 +87,7 @@ protected GearGroupResource(final String uuid, final String name, final Collecti
* @param servicethe underlying REST Service
*/
protected GearGroupResource(final GearGroupResourceDTO dto, final ApplicationResource application, final IRestService service) {
this(dto.getUuid(), dto.getName(), dto.getGears(), dto.getCartridges(), application, service);
this(dto.getUuid(), dto.getName(), dto.getGears(), dto.getCartridges(), dto.getAdditionalStorage(), application, service);
}

public final String getUUID() {
Expand All @@ -92,11 +97,15 @@ public final String getUUID() {
public final String getName() {
return name;
}

public Collection<IGear> getGears() {
return Collections.unmodifiableCollection(gears);
}

public int getAdditionalStorage() {
return additionalStorage;
}

/**
* @return the cartridges
*/
Expand Down
Loading