Skip to content
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

Dynamic AMI selection: fix architecture filter #283

Merged
merged 5 commits into from
Jul 30, 2024
Merged

Dynamic AMI selection: fix architecture filter #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2cb71d2
Fix architecture, add volume size filter
brunoasr Jul 15, 2024
ae7bbe3
Remove volume size filter
brunoasr Jul 16, 2024
5956092
Remove volume size filter
brunoasr Jul 16, 2024
84a1b94
Fix environment selection
brunoasr Jul 16, 2024
3d9360f
Remove print
brunoasr Jul 16, 2024
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
11 changes: 5 additions & 6 deletions orion-server/src/main/java/com/pinterest/orion/core/actions/aws/AmiTagManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import software.amazon.awssdk.services.ec2.model.CreateTagsRequest;
import software.amazon.awssdk.services.ec2.model.CreateTagsResponse;
import software.amazon.awssdk.services.ec2.model.Filter;
import software.amazon.awssdk.services.ec2.model.Image;
import software.amazon.awssdk.services.ec2.model.Tag;

/**
Expand All @@ -50,7 +49,7 @@ public class AmiTagManager {
public static final String KEY_AMI_ID = "ami_id";
public static final String KEY_APPLICATION = "application";
public static final String KEY_RELEASE = "release";
public static final String KEY_CPU_ARCHITECTURE = "cpu_architecture";
public static final String KEY_ARCHITECTURE = "architecture";
public static final String KEY_APPLICATION_ENVIRONMENT = "application_environment";
public static final String VALUE_KAFKA = "kafka";
public static UnaryOperator<String> tag = key -> "tag:" + key;
Expand Down Expand Up @@ -82,10 +81,10 @@ public List<Ami> getAmiList(Map<String, String> filter) {
.values(filter.get(KEY_RELEASE))
.build()
);
if (filter.containsKey(KEY_CPU_ARCHITECTURE))
if (filter.containsKey(KEY_ARCHITECTURE))
filterList.add(
filterBuilder.name(tag.apply(KEY_CPU_ARCHITECTURE))
.values(filter.get(KEY_CPU_ARCHITECTURE))
filterBuilder.name(KEY_ARCHITECTURE)
.values(filter.get(KEY_ARCHITECTURE))
.build()
);
filterList.add(
Expand All @@ -98,7 +97,7 @@ public List<Ami> getAmiList(Map<String, String> filter) {
DescribeImagesResponse resp = ec2Client.describeImages(builder.build());
if (resp.hasImages() && !resp.images().isEmpty()) {
// The limitation of images newer than 180 days is temporarily suspended
//ZonedDateTime cutDate = ZonedDateTime.now().minusDays(180);
// ZonedDateTime cutDate = ZonedDateTime.now().minusDays(180);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean we are including all the images now? What is the rationale behind 180 days?
If we want to add a threshold like that I would suggest making it a configuration so it can be set as needed without having to change code.

resp.images().forEach(image -> {
/*if (ZonedDateTime.parse(image.creationDate(), DateTimeFormatter.ISO_ZONED_DATE_TIME).isAfter(cutDate)) {*/
Iterator<Tag> i = image.tags().iterator();
Expand Down
2 changes: 1 addition & 1 deletion orion-server/src/main/java/com/pinterest/orion/server/api/Ami.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class Ami {
*
* @param amiId - for AWS, the string "ami-" and a sequence of 17 characters
* @param applicationEnvironment - comma-separated list of environments (dev, test,
* staging, prod) supported by this ami
* stage, prod, m10n) supported by this ami
* @param creationDate - ami creation date, UTC
* @return the new Ami instance
*/
Expand Down
4 changes: 2 additions & 2 deletions orion-server/src/main/java/com/pinterest/orion/server/api/ClusterManagerApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ public Map<String, Map<String, Utilization>> getUtilizationDetailsByCluster() {
@GET
public List<Ami> describeImages(
@QueryParam(AmiTagManager.KEY_RELEASE) String os,
@QueryParam(AmiTagManager.KEY_CPU_ARCHITECTURE) String arch
@QueryParam(AmiTagManager.KEY_ARCHITECTURE) String arch
) {
Map<String, String> filter = new HashMap<>();
if (os != null)
filter.put(AmiTagManager.KEY_RELEASE, os);
if (arch != null)
filter.put(AmiTagManager.KEY_CPU_ARCHITECTURE, arch);
filter.put(AmiTagManager.KEY_ARCHITECTURE, arch);
if (amiTagManager == null)
amiTagManager = new AmiTagManager();
return amiTagManager.getAmiList(filter);
Expand Down
35 changes: 19 additions & 16 deletions orion-server/src/main/resources/webapp/src/basic-components/Ami.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,30 @@ function Ami({ amiList, requestAmiList, envTypes, requestEnvTypes, updateAmiTag
const handleOSChange = event => {
setOS(event.target.value);
};
const [cpuArch, setCPUArch] = React.useState();
const handleCPUArchChange = event => {
setCPUArch(event.target.value);
const [arch, setArch] = React.useState();
const handleArchChange = event => {
setArch(event.target.value);
};
const [selected, setSelected] = React.useState([]);
const [env, setEnv] = React.useState({});
if (envTypes !== undefined && Object.keys(env).length == 0) {
const targetEnv = {};
envTypes.forEach(value => { targetEnv[value] = false; });
setEnv(targetEnv);
}
const handleTableRowSelect = (id, row) => {
setSelected(id);
setAppEnv(row.applicationEnvironment);
const envs_str = row.applicationEnvironment;
const envs = envs_str.split(',');
env.dev = env.test = env.staging = env.prod = false;
envTypes.forEach(envType => { env[envType] = false; });
for (const env_str of envs)
env[env_str] = true;
};
const [appEnv, setAppEnv] = React.useState();
const handleAppEnvChange = event => {
setAppEnv(event.target.value);
};
const envMap = {};
if (envTypes !== undefined)
envTypes.forEach(value => { envMap[value] = false; });
const [env] = React.useState(envMap);
const handleCheckboxChange = (event) => {
env[event.target.name] = event.target.checked;
const newAppEnv = [];
Expand All @@ -81,8 +83,8 @@ function Ami({ amiList, requestAmiList, envTypes, requestEnvTypes, updateAmiTag
const parms = [];
if (os)
parms.push("release=" + os);
if (cpuArch)
parms.push("cpu_architecture=" + cpuArch);
if (arch)
parms.push("architecture=" + arch);
requestAmiList(parms.join('&'));
requestEnvTypes();
}
Expand Down Expand Up @@ -121,20 +123,21 @@ function Ami({ amiList, requestAmiList, envTypes, requestEnvTypes, updateAmiTag
>
<MenuItem value={"bionic"}>bionic</MenuItem>
<MenuItem value={"focal"}>focal</MenuItem>
<MenuItem value={"noble"}>noble</MenuItem>
</Select>
</FormControl>
</div>
<div>
<FormControl className={classes.formControl}>
<InputLabel id="lblCPUArch">CPU Architecture</InputLabel>
<InputLabel id="lblArch">architecture</InputLabel>
<Select
labelId="lblSelectCPUArch"
id="selectCPUArch"
value={cpuArch}
onChange={handleCPUArchChange}
labelId="lblSelectArch"
id="selectArch"
value={arch}
onChange={handleArchChange}
style={{ width: "200px", textAlign: "left" }}
>
<MenuItem value={"amd64"}>amd64</MenuItem>
<MenuItem value={"x86_64"}>x86_64</MenuItem>
<MenuItem value={"arm64"}>arm64</MenuItem>
</Select>
</FormControl>
Expand Down
Loading