-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add V2 scope to /api/clusters endpoint #1492
Conversation
@@ -135,7 +130,7 @@ defmodule TrentoWeb.OpenApi.V1.Schema.Cluster do | |||
type: %Schema{ | |||
type: :string, | |||
description: "Detected type of the cluster", | |||
enum: ClusterType.values() | |||
enum: [:hana_scale_up, :hana_scale_out, :unknown] |
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.
Using hardcoded values as they cannot be changed without adding non backward compatible changes.
This schema won't be updated anymore
@@ -22,30 +23,12 @@ defmodule TrentoWeb.V1.ClusterView do | |||
|> Map.put(:id, data.cluster_id) | |||
end | |||
|
|||
def render("settings.json", %{settings: settings}) do |
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.
Taking advantage to remove old unused code
assets/js/lib/network/index.js
Outdated
@@ -68,36 +68,36 @@ function handleResponseStatus(response) { | |||
return response; | |||
} | |||
|
|||
export const post = function post(url, data) { | |||
export const post = function post(url, data, config) { |
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.
We can pass the config to overwrite the existing one, in our case the baseURL
.
If an empty config is provided, the default one is used:
https://axios-http.com/docs/instance
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.
Do you think that for readability we could have the config with a default argument of null or something at least compatible with the "empty config" just to match the arity of the function without explicitly ignore the argument
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.
Wow, great job man! 🚀
I just noted down a few adjustments that might be helpful.
}, | ||
roles: %Schema{ | ||
type: :array, | ||
items: %Schema{type: :string, enum: ["ascs", "ers"]}, |
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 mean that we can have at most two roles, right?
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.
Yes, one node can have from 0 to 2 roles for each SAP system.
The normal scenario is when one node is only ascs
and the other ers
, but during a failover, one nodes takes both roles and the other none.
PD: Creating a tech-debt ticket to put the enum values in an Enum
object, so we don't need to spread them all around the code
@@ -119,7 +119,9 @@ function* initialDataFetch() { | |||
yield put(stopHostsLoading()); | |||
|
|||
yield put(startClustersLoading()); | |||
const { data: clusters } = yield call(get, '/clusters'); | |||
const { data: clusters } = yield call(get, '/clusters', { |
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 guess for now this is enough.
However at a certain point I'd find it of great help having something like yield call(getV2, '/clusters');
Feel free to keep it as you did for now.
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.
Fair enough. I wanted to keep the current changes minimalist.
This wouldn't require many changes, simply to add 4 new functions (get, post, del, put) with that naming.
Let's keep it on the radar
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.
Thanks @arbulu89!
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.
Just a minor comment but LGTM
fa1597f
to
1839f40
Compare
* Add openapi v2 schemas * Add clusters v2 endpoint controller and view * Update pheonix router to add v2 endpoints * Adapt clusters v1 schema and view to be backward compatible * Use clusters v2 version on projector * Remove deprecated code * Use /api/v2/clusters endpoint in frontend * Improve and fix some new schema implementations
* Add openapi v2 schemas * Add clusters v2 endpoint controller and view * Update pheonix router to add v2 endpoints * Adapt clusters v1 schema and view to be backward compatible * Use clusters v2 version on projector * Remove deprecated code * Use /api/v2/clusters endpoint in frontend * Improve and fix some new schema implementations
* Host deregistration events (#1245) * Host deregistration commands * Host deregistration events * Fixing module attributes in host deregistration commands * Fix deregistration commands namespace * Deregistration side effects (#1252) * Add deregistered_at to read model Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com> Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com> * Handle side effects from HostDeregistered event Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com> Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com> * Filter deregistered hosts from get_all_hosts Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com> Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com> * Fix tests Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com> Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Add test for filtering unregistered hosts Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com> Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Host Deregistration process manager and aggregate handling (#1249) * Host aggregate handling of deregistration event * Wip Deregistration process manager * Process Managers supervisor * Add Deregistration process manager test * Fix credo and mispells * Squashed Process manager state into process manager * Routing deregistration commands to Host aggregate * Fix deregistration process manager state with deftype * Format router file * Deregistration timestamp carried from Deregistration requested command * Fix spell in process manager docstring * Fix typo in test description * Cluster and cluster host deregistration events/commands (#1274) * Cluster and cluster host deregistration events/commands * Address comments * Remove leading space * Fix Hosts query without the soft deleted hosts exclusion (#1282) * Cluster registration also for nodes not DC (#1275) * Cluster registration process happens also when the discovered node is not a DC * Address docs and testing review feedbacks * Removed useless tests from cluster_tests * Add host to cluster when a message from a dc host arrives for a cluster Co-Authored-By: Fabrizio Sestito <fabrizio.sestito@suse.com> * Add register cluster host test when the host is and is not a DC --------- Co-authored-by: Fabrizio Sestito <fabrizio.sestito@suse.com> * Cluster deregistration process manager & aggregate changes (#1278) * Make Process Manager aware of cluster deregistration Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Handle cluster host deregistration in the cluster aggregate Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Route the DeregisterClusterHost to the cluster aggregate --------- Co-authored-by: Jamie Rodriguez <jamie.rodriguez@suse.com> * Cluster deregistered side effects (#1280) * Add deregistered_at field to Cluster read model * ClusterDeregistered event projection * Wip on clusters module, get clusters that are not soft deleted * Clusters entrypoint tests * Test broadcast of cluster deregistration in cluster projector * Removed first from request_check_execution query * Fix query parenthesis in request_clusters_checks_execution * get_all_clusters, tested for deregistered and soft deleted clusters * Handle HostRemovedFromCluster in host projection * Cluster projector reorder of project clauses * Host projector set the cluster to null when HostRemovedFromCluster * Correct typos (#1315) * Database registration changes (#1326) * Database sap system registration change The database istance is register only if the sr is disabled or with sr enabled but only primary instances * Sap systems moduledoc updated * Changed error when registering a non primary database for a sap system * Updated moduledoc of sap system domain * Refuse database registration changes The registration is refused if the sap system does not exists and the database has a secondary role * Fix sap system domain tests * Fix typos * Format * Add SAP System deregistration commands & events (#1314) Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Fix Deregistration process manager state validation * Add application instance deregistration logic (#1357) Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com> * Application instances registration changes (#1358) * Add SAP System de/registration logic to Process Manager (#1356) Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Add database instance deregistration logic (#1372) Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com> Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Add SAP System deregistration side effects (#1387) Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Detect ascs/ers cluster type (#1392) * Detect ascs/ers cluster type * Update functions order to improve pattern matching * Ascs ers type frontend (#1398) * Add ascs/ers cluster type to the frontend * Improve e2e testing of the clusters view * Discover ASCS/ERS cluster SIDs (#1400) * Discover ascs/ers cluster sids * Add additional_sids to domain and side effects * Add additional_sids to cluster api schema * Handle `:database_not_registered` errors (#1405) Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Add polymorphic embed usage to deftype macro (#1411) * Properly handle multiple errors returned (#1413) * Display ASCS/ERS clusters SID in clusters view (#1403) * Show additional sids in the clusters view * Adapt e2e tests to check for sids * Order get_all_clusters output by id * Revert HeartbeatSucceeded to old name (#1429) * Add deregistration websocket events (#1424) Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com> Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Discover ASCS/ERS clusters health and initial details (#1422) * Add nodes entry to the crm payload * Add ascs/ers cluster details value objects * Implement ascs/ers cluster health discovery * Update cluster details to the polymorphic embed * Add tests to the ascs/ers cluster health discovery * Fix validate_required usage * Set default value to additional_sids in cluster events (#1441) * Cluster delta deregistration (#1386) * Add get_cluster_id_by_host_id/1 * Add delta deregistration logic * Add additional ascs/ers cluster details discovery (#1442) * Add additional ascs/ers cluster details discovery * Add comment to parse_ascs_ers_cluster_nodes * Wrap long function in do/end * Set false instead of nil in Enum.filter function * Deregistration process manager retry the rollup errors from aggregates (#1473) * Fix HostRemovedFromCluster concurrency issues (#1474) * Basic deregistration leftovers (#1461) * test * Add deregistered_at field to Database * Fix update of Application and Database when deregistering instances * Put deregistered_at timestamp instead of nil sid on database deregistration * Remove sid to nil when Sap system is deregistered * Emit sap system deregistered when the database is deregistered * Emit database deregistered event when no instances left and db is not already deregistered * Sap systems domain deregistration tests refactored * format sap system domain * Emit database deregistered event when they are instances only when the db is not registered * Removed redundant nil default to deregistered_at in database * Fix mispell in sap system tests * Add fake_sid util to sap system testing suite * Sap system does not get deregistered a second time * Changed test titles in sap system test, to remove "a second time" misleading phrase * Deregistration tombstoning rollup (#1425) * Host tombstoning on host deregistration * Add host tombstone rollup handling in stream event handler * Cluster tombstoned event and lifespan when cluster is deregistered * Cluster rollup triggered on Cluster tombstone event * Move cluster tombstoned event to cluster domain folder * Sap system aggregate emits tombstone event * Sap system aggregate tombstoning test * SapSystem tombstoned event triggers sap system rollup * Removed tombstoned events from lifespan stop mechanism in all aggregates * Formatted Cluster/Host tombstoned events * Fix logging string in stream rollup event handler * Addressing review feedbacks * Credo format * Change redirect plug to use a list of available versions (#1487) * Change redirect plug to use a list of available versions * Add sanity check for empty available api versions list * Use Keyword.get to get the opts * Enable openapi schema versioning (#1488) * Convert the open api plug into a versinable macro * Move V1 schemas to its own folder * Update controllers to used versioned openapi * Update router to pipethrough specific open api version * Disable open api cache on dev * Update controller tests to use new openapi versions * Temporarily set openapi V1 version in CI docs generation * Test paths generation in open api spec * Add endpoint to deregister a host (#1450) Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> * Add V2 scope to /api/clusters endpoint (#1492) * Add openapi v2 schemas * Add clusters v2 endpoint controller and view * Update pheonix router to add v2 endpoints * Adapt clusters v1 schema and view to be backward compatible * Use clusters v2 version on projector * Remove deprecated code * Use /api/v2/clusters endpoint in frontend * Improve and fix some new schema implementations * Aggregate clauses reorder (#1495) * Reorder host aggregate clauses, host_id: nil clauses compacted * Sap system clauses sap_system_id: nil, compacted * Sap system delta deregistration (#1491) * Fix cluster delta computation * Add get_application_instances_by_host_id/1 and get_database_instances_by_host_id/1 * Add delta deregistration logic * Identify sap systems ensa version (#1496) * Identify sap systems ensa version * Replace nil by no_ensa atom in EnsaVersion enum * DatabaseDeregistered side effects (#1504) * Add deregistered_at field to databases read model * Handle database deregistered side effects * Filtered deregistered databases in the usecases * Make `HostReadModel.last_heartbeat_timestamp` a virtual field (#1505) * Project removal of db/app instances to the read models (#1507) * Project removal of db/app instances to the read models * Reject commands when aggregate deregistered (#1513) * Host aggregate reject commands when the host is deregistered * Cluster aggregate reject all commands except registration when deregistered * Sap system aggregate rejects commands when deregistered * Host aggregate rejects updates when deregistered * Cluster aggregate rejects all commands when deregistered * Fix host tests * Ensa version domain (#1512) * Add ensa_version to SapSystemReadModel * Add SapSystemUpdated event and domain logic * Add and update sap system domain tests * Add SapSystemUpdated projection * Add new ensa_version field in openapi schema * Add test to check updated event is not emitted * Update migration to set default value afterwards * Create AscsErsClusterRole enum (#1520) * Host restoration (#1517) * Host restoration in Host aggregate * Host restoration projector * Addressing typo feedbacks * Move host details update after dergistered_at guard clause * Broadcast sap system updated (#1516) * Broadcast SapSystemUpdated event * Update sap systems in redux state upon update event * Cluster restoration (#1523) * Extract cluster update procedure to separate function * Cluster restoration * Cluster restored projection * Addressing review feedbacks * Addressing review feedbacks * Move sagas functions to separate files according to adr 0009 (#1531) * Update health summary endpoint (#1530) * Update health summary service and view * Update openapi schema and deprecate old fields * Polish the code and handle unclustered systems * Fix function specs * Remove fixed sid usage from db instance creation in test * Frontend instance removal on broadcast (#1522) * Add sid to instance deregistration broadcast * Remove instances from upon instance deregistration broadcast * Apply various cleanups to the database state * Use Repo.get! where possible * Improve notification message * Fix wrong test placement * Add test for instances removal notify * Fix sap system domain apply ordering issues (#1550) * Move application instances (#1554) * Add Deregistration modal (#1537) * Allow rollup on deregistered aggregates (#1551) * Complete tests on command acceptance/rejection (#1563) * Sap system restoration (#1545) * Database restoration in sap system aggregate * Database restore projection * Sap system restore domain * Sap system restore projection * Emit application instance registered or moved in restore * Removed health multi change on sap system restoration * Remove sap_system_id setting in database restored apply * Removed sid from DatabaseRestored event * Removed sid field from SapSytemRestored event --------- Co-authored-by: Carmine Di Monaco <carmine.dimonaco@suse.com> Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com> Co-authored-by: Fabrizio Sestito <fabrizio.sestito@suse.com> Co-authored-by: Jamie Rodriguez <jamie.rodriguez@suse.com> Co-authored-by: Carmine Di Monaco <carmine.dimonaco@gmail.com> Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com> Co-authored-by: Eugen Maksymenko <eugen.maksymenko@suse.com> Co-authored-by: Jurgen Goldschmidt <jurgen.goldschmidt@suse.com>
Description
Follow up for: #1468
Include the
v2
scope for the/api/clusters
endpoint.The
v1
has been updated to be backward compatible.I wanted to create a new PR with the frontend changes... but without them the e2e tests fail. I hope it is not that much...
How was this tested?
Tested