Skip to content

Added ReadonlyRootfs option. #245

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

Merged
marcuslinke merged 1 commit into from
Jun 16, 2015
Merged

Added ReadonlyRootfs option. #245

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
16 changes: 14 additions & 2 deletions src/main/java/com/github/dockerjava/api/model/HostConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public class HostConfig {
@JsonProperty("Privileged")
private boolean privileged;

@JsonProperty("ReadonlyRootfs")
private boolean readonlyRootfs;
Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Jun 16, 2015

Choose a reason for hiding this comment

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

boolean of Boolean?

Copy link
Copy Markdown
Contributor

@marcuslinke marcuslinke Jun 16, 2015

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Jun 16, 2015

Choose a reason for hiding this comment

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

Can't this cause problems when property wouldn't exist? Boolean will default to null, while boolean may report false negative 'false'.

Copy link
Copy Markdown
Contributor

@marcuslinke marcuslinke Jun 16, 2015

Choose a reason for hiding this comment

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

Good point. I think we should change all json mapped boolean fields into Boolean in a separate refactoring.

Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Jun 16, 2015

Choose a reason for hiding this comment

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

I just interesting because i see inconsistency everywhere. Maybe document somewhere DESIGN.md ?

Copy link
Copy Markdown
Contributor

@marcuslinke marcuslinke Jun 16, 2015

Choose a reason for hiding this comment

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

What inconsistencies did you find. Some examples?

Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Jun 16, 2015

Choose a reason for hiding this comment

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

boolean vs Boolean in other places, but i may mistake.

Copy link
Copy Markdown
Contributor Author

@scadgek scadgek Jun 17, 2015

Choose a reason for hiding this comment

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

Well, in this case a default boolean = false will be ok as long as ReadonlyRootfs option is false by default. I think it'll we better to add default values in such classes corresponding to docker defaults, than creating objects and handling nulls in several places.

Copy link
Copy Markdown
Contributor

@marcuslinke marcuslinke Jun 17, 2015

Choose a reason for hiding this comment

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

Best will be to use object types instead of primitives. See #246


@JsonProperty("Dns")
private String[] dns;

Expand Down Expand Up @@ -64,7 +67,7 @@ public HostConfig() {
}

public HostConfig(Bind[] binds, Link[] links, LxcConf[] lxcConf, Ports portBindings, boolean publishAllPorts,
boolean privileged, String[] dns, String[] dnsSearch, VolumesFrom[] volumesFrom, String containerIDFile,
boolean privileged, boolean readonlyRootfs, String[] dns, String[] dnsSearch, VolumesFrom[] volumesFrom, String containerIDFile,
Capability[] capAdd, Capability[] capDrop, RestartPolicy restartPolicy, String networkMode, Device[] devices,
String[] extraHosts, Ulimit[] ulimits) {
this.binds = new Binds(binds);
Expand All @@ -73,6 +76,7 @@ public HostConfig(Bind[] binds, Link[] links, LxcConf[] lxcConf, Ports portBindi
this.portBindings = portBindings;
this.publishAllPorts = publishAllPorts;
this.privileged = privileged;
this.readonlyRootfs = readonlyRootfs;
this.dns = dns;
this.dnsSearch = dnsSearch;
this.volumesFrom = volumesFrom;
Expand All @@ -95,7 +99,7 @@ public Bind[] getBinds() {
public LxcConf[] getLxcConf() {
return lxcConf;
}

public Ports getPortBindings() {
return portBindings;
}
Expand All @@ -108,6 +112,10 @@ public boolean isPrivileged() {
return privileged;
}

public boolean isReadonlyRootfs() {
return readonlyRootfs;
}

public String[] getDns() {
return dns;
}
Expand Down Expand Up @@ -183,6 +191,10 @@ public void setPrivileged(boolean privileged) {
this.privileged = privileged;
}

public void setReadonlyRootfs(boolean readonlyRootfs) {
this.readonlyRootfs = readonlyRootfs;
}

public void setDns(String[] dns) {
this.dns = dns;
}
Expand Down