RDKEMW-15105: Let's get connected" screen displayed during Migration testing#288
RDKEMW-15105: Let's get connected" screen displayed during Migration testing#288gururaajar wants to merge 17 commits intosupport/1.12.0from
Conversation
…testing Reason for Change: Deleting all the connections only in case of BOOT_MIGRATION so that networkmanager connecting to the eth0 connection can be avoided and we can create new "Wired Connection 1" to connect. Test Procedure: Migration Scenario Priority: P1 Risks: Medium Signed-off-by: Gururaaja ESR<Gururaja_ErodeSriranganRamlingham@comcast.com>
…testing Reason for Change: Deleting all the connections only in case of BOOT_MIGRATION so that networkmanager connecting to the eth0 connection can be avoided and we can create new "Wired Connection 1" to connect. Test Procedure: Migration Scenario Priority: P1 Risks: Medium Signed-off-by: Gururaaja ESR<Gururaja_ErodeSriranganRamlingham@comcast.com>
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent the “Let’s get connected” screen during migration testing by deleting existing NetworkManager connections only on migration boot, so a fresh “Wired connection 1” can be created/activated without NetworkManager auto-connecting to an existing eth0 profile.
Changes:
- Add an async delete callback for NetworkManager connections.
- On interface enable, read
/tmp/bootTypeand (if migration boot) delete all NM connections before activating the target interface’s known connection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // Check boot type and delete all ethernet NM connections if BOOT_MIGRATION | ||
| { | ||
| const char* bootFile = "/tmp/bootType"; | ||
| std::ifstream file(bootFile); | ||
|
|
||
| if(file.is_open()) | ||
| { | ||
| std::string line, bootTypeValue; | ||
| while(std::getline(file, line)) | ||
| { | ||
| const std::string key = "BOOT_TYPE="; | ||
| auto pos = line.find(key); | ||
| if(pos != std::string::npos) | ||
| { | ||
| bootTypeValue = line.substr(pos + key.size()); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if(bootTypeValue == "BOOT_MIGRATION") | ||
| { | ||
| NMLOG_INFO("BOOT_MIGRATION detected, deleting all wired NM connections"); | ||
|
|
||
| const GPtrArray *connections = nm_client_get_connections(client); | ||
| if(connections && connections->len > 0) | ||
| { | ||
| /* Snapshot the list before iterating: nm_client_get_connections() | ||
| * returns an internal array that can be mutated as connections | ||
| * are removed, so we must not iterate it while deleting. */ | ||
| GPtrArray *snapshot = g_ptr_array_new_full(connections->len, g_object_unref); | ||
| for(guint i = 0; i < connections->len; ++i) | ||
| { | ||
| NMRemoteConnection *conn = NM_REMOTE_CONNECTION(connections->pdata[i]); | ||
| if(!conn) continue; | ||
| NMSettingConnection *sCon = nm_connection_get_setting_connection(NM_CONNECTION(conn)); | ||
| if(!sCon) continue; | ||
| const char *connType = nm_setting_connection_get_connection_type(sCon); | ||
| if(g_strcmp0(connType, NM_SETTING_WIRED_SETTING_NAME) != 0) | ||
| { | ||
| NMLOG_DEBUG("Skipping non-wired connection type: %s", connType ? connType : "null"); | ||
| continue; | ||
| } | ||
| g_ptr_array_add(snapshot, g_object_ref(conn)); | ||
| } | ||
|
|
||
| for(guint i = 0; i < snapshot->len; ++i) | ||
| { | ||
| NMRemoteConnection *conn = NM_REMOTE_CONNECTION(snapshot->pdata[i]); | ||
| GError *error = nullptr; | ||
| if(!nm_remote_connection_delete(conn, nullptr, &error)) | ||
| { | ||
| const char *connId = nm_connection_get_id(NM_CONNECTION(conn)); | ||
| NMLOG_ERROR("Failed to delete connection %s: %s", | ||
| connId ? connId : "<unknown>", | ||
| error ? error->message : "unknown error"); | ||
| if(error) g_error_free(error); | ||
| } | ||
| } | ||
| g_ptr_array_unref(snapshot); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
BOOT_MIGRATION marker file (/tmp/bootType) is read but never removed. This breaks the intended idempotency (and will cause repeated wired-connection deletions on subsequent SetInterfaceState(enabled=true) calls as long as the file exists), and it also conflicts with the new L2 test expectation that the production code removes the marker. Consider deleting the marker after handling BOOT_MIGRATION (and logging if the remove fails) so the migration cleanup runs only once.
| struct stat bootFileStat; | ||
| int rc = stat("/tmp/bootType", &bootFileStat); | ||
| EXPECT_EQ(-1, rc); | ||
| if (rc == -1) | ||
| EXPECT_EQ(ENOENT, errno); | ||
| } |
There was a problem hiding this comment.
This test checks errno (ENOENT) after stat(), but the file does not include <cerrno>/<errno.h>. Relying on transitive includes for errno is brittle and can break compilation depending on header ordering/toolchain; include the proper header explicitly.
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| GError *error = nullptr; | ||
| if(!nm_remote_connection_delete(conn, nullptr, &error)) | ||
| { | ||
| const char *connId = nm_connection_get_id(NM_CONNECTION(conn)); | ||
| NMLOG_ERROR("Failed to delete connection %s: %s", | ||
| connId ? connId : "<unknown>", | ||
| error ? error->message : "unknown error"); | ||
| if(error) g_error_free(error); | ||
| } | ||
| NMLOG_INFO("Deleted the conn %s", connId ? connId : "<unknown>"); |
There was a problem hiding this comment.
connId is declared inside the error branch but is used unconditionally afterward in the info log, which will not compile (out-of-scope) and also makes the success-path log incorrect. Capture the connection ID in a local variable before calling nm_remote_connection_delete() (with a safe fallback when NULL), then use it for both error and success logs.
| GError *error = nullptr; | |
| if(!nm_remote_connection_delete(conn, nullptr, &error)) | |
| { | |
| const char *connId = nm_connection_get_id(NM_CONNECTION(conn)); | |
| NMLOG_ERROR("Failed to delete connection %s: %s", | |
| connId ? connId : "<unknown>", | |
| error ? error->message : "unknown error"); | |
| if(error) g_error_free(error); | |
| } | |
| NMLOG_INFO("Deleted the conn %s", connId ? connId : "<unknown>"); | |
| const char *connId = nm_connection_get_id(NM_CONNECTION(conn)); | |
| if(!connId) | |
| connId = "<unknown>"; | |
| GError *error = nullptr; | |
| if(!nm_remote_connection_delete(conn, nullptr, &error)) | |
| { | |
| NMLOG_ERROR("Failed to delete connection %s: %s", | |
| connId, | |
| error ? error->message : "unknown error"); | |
| if(error) g_error_free(error); | |
| } | |
| else | |
| { | |
| NMLOG_INFO("Deleted the conn %s", connId); | |
| } |
| const char* bootFile = "/tmp/bootType"; | ||
| std::ifstream file(bootFile); | ||
|
|
||
| if(file.is_open()) | ||
| { | ||
| std::string line, bootTypeValue; | ||
| while(std::getline(file, line)) | ||
| { | ||
| const std::string key = "BOOT_TYPE="; | ||
| auto pos = line.find(key); | ||
| if(pos != std::string::npos) | ||
| { | ||
| bootTypeValue = line.substr(pos + key.size()); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if(bootTypeValue == "BOOT_MIGRATION") | ||
| { | ||
| NMLOG_INFO("BOOT_MIGRATION detected, deleting all wired NM connections"); | ||
|
|
||
| // Bring down the ethernet interface before wiping its connections | ||
| // so NM doesn't immediately re-activate them during deletion. | ||
| NMDevice *ethDev = nm_client_get_device_by_iface(client, interface.c_str()); | ||
| if(ethDev) | ||
| { | ||
| GError *discError = nullptr; | ||
| if(!nm_device_disconnect(ethDev, nullptr, &discError)) | ||
| { | ||
| NMLOG_WARNING("Failed to disconnect %s before migration cleanup: %s", | ||
| interface.c_str(), | ||
| discError ? discError->message : "unknown error"); | ||
| if(discError) g_error_free(discError); | ||
| } | ||
| NMLOG_INFO("nm_device_disconnect successful"); | ||
| } | ||
|
|
||
| const GPtrArray *connections = nm_client_get_connections(client); | ||
| if(connections && connections->len > 0) | ||
| { | ||
| /* Snapshot the list before iterating: nm_client_get_connections() | ||
| * returns an internal array that can be mutated as connections | ||
| * are removed, so we must not iterate it while deleting. */ | ||
| GPtrArray *snapshot = g_ptr_array_new_full(connections->len, g_object_unref); | ||
| for(guint i = 0; i < connections->len; ++i) | ||
| { | ||
| NMRemoteConnection *conn = NM_REMOTE_CONNECTION(connections->pdata[i]); | ||
| if(!conn) continue; | ||
| NMSettingConnection *sCon = nm_connection_get_setting_connection(NM_CONNECTION(conn)); | ||
| if(!sCon) continue; | ||
| const char *connType = nm_setting_connection_get_connection_type(sCon); | ||
| if(g_strcmp0(connType, NM_SETTING_WIRED_SETTING_NAME) != 0) | ||
| { | ||
| NMLOG_DEBUG("Skipping non-wired connection type: %s", connType ? connType : "null"); | ||
| continue; | ||
| } | ||
| g_ptr_array_add(snapshot, g_object_ref(conn)); | ||
| } | ||
|
|
||
| for(guint i = 0; i < snapshot->len; ++i) | ||
| { | ||
| NMRemoteConnection *conn = NM_REMOTE_CONNECTION(snapshot->pdata[i]); | ||
| GError *error = nullptr; | ||
| if(!nm_remote_connection_delete(conn, nullptr, &error)) | ||
| { | ||
| const char *connId = nm_connection_get_id(NM_CONNECTION(conn)); | ||
| NMLOG_ERROR("Failed to delete connection %s: %s", | ||
| connId ? connId : "<unknown>", | ||
| error ? error->message : "unknown error"); | ||
| if(error) g_error_free(error); | ||
| } | ||
| NMLOG_INFO("Deleted the conn %s", connId ? connId : "<unknown>"); | ||
| } | ||
| g_ptr_array_unref(snapshot); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
/tmp/bootType is read to trigger BOOT_MIGRATION cleanup, but it is never removed afterward. This makes the behavior non-idempotent (cleanup will re-run on every enable) and contradicts the L2 test that asserts the marker file is deleted. After successfully detecting/handling BOOT_MIGRATION, remove the marker file and log/handle failures (e.g., check std::remove() return and errno).
…kmanager into topic/RDKEMW-15105_8.3
…kmanager into topic/RDKEMW-15105_8.3
Reason for Change: Deleting all the connections only in case of BOOT_MIGRATION so that networkmanager connecting to the eth0 connection can be avoided and we can create new "Wired Connection 1" to connect.
Test Procedure: Migration Scenario
Priority: P1
Risks: Medium
Signed-off-by: Gururaaja ESRGururaja_ErodeSriranganRamlingham@comcast.com