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.

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

Trace Paths in Entrypoint #395

Conversation

ingvaldlorentzen
Copy link

New Behavior

Adds a new script (trace_paths) to the netbox container entrypoint.

The fact that netbox-docker currently does not call this script broke cable traces in our environment after an upgrade to NetBox v2.10.0 due to changes in the cable tracing logic.

Contrast to Current Behavior

The script is currently not called in entrypoint or any other place in netbox-docker as far as I can see.

Discussion: Benefits and Drawbacks

The script will run on every container start, this is probably a bit overkill, especially in K8s scenarios. However, the script should only retrace missing cable traces, so the overhead shouldn't be too big.
If you have suggestions for a different location, I'm open for it, but since it's doing DB operations I'm not sure where else it would fit

The netbox upgrade script also calls remove_stale_contenttypes, clearsessions, and the cache invalidate all.
Maybe these should be added to the entrypoint as well? I think this would solve a lot of issues related to upgrading the netbox-docker image in an environment.
I don't think these are as critical as trace_paths, but I'll gladly add them to this PR if you agree.

Changes to the Wiki

I don't think anything is necessary here, but adding a "How to upgrade" wiki page might be something worth considering.

Proposed Release Note Entry

The NetBox manage.py command trace_paths from the NetBox upgrade script is now called in docker entrypoint

Double Check

  • I have read the comments and followed the PR template.
  • I have explained my PR according to the information in the comments.
  • My PR targets the develop branch.

@ingvaldlorentzen ingvaldlorentzen changed the title - Added trace_paths to entrypoint Trace Paths in Entrypoint Jan 18, 2021
@cimnine cimnine added the discussion This issue requires further input from the community. label Jan 20, 2021
@cimnine
Copy link
Collaborator

cimnine commented Jan 27, 2021

Thank you for your contribution. We're sorry we're not able to provide feedback sooner. Currently our focus lies on releasing the next version. Then we'll get back to you on this suggestion.

@jhujhiti
Copy link
Contributor

jhujhiti commented Feb 3, 2021

It's also important that we invalidate the cache after tracing the paths, otherwise the traced paths will not appear. Of course, flushing the cache on every startup is not ideal...

@cimnine
Copy link
Collaborator

cimnine commented Feb 27, 2021

I get where the idea to integrate this comes from. But these are only relevant during upgrades. And tracing paths was required once because the DB model changed, wasn't it?

But I agree that a better upgrade management would be nice. Yet it would be non-trivial to implement reliably.

@tigpas
Copy link

tigpas commented Apr 20, 2021

I found this solution for me

---  docker/docker-entrypoint.sh
+++ docker/docker-entrypoint.sh
@@ -15,7 +15,7 @@
 DB_WAIT_TIMEOUT=${DB_WAIT_TIMEOUT-3}
 MAX_DB_WAIT_TIME=${MAX_DB_WAIT_TIME-30}
 CUR_DB_WAIT_TIME=0
-while ! ./manage.py migrate 2>&1 && [ "${CUR_DB_WAIT_TIME}" -lt "${MAX_DB_WAIT_TIME}" ]; do
+while ! SHOWMIGRATIONS="$(./manage.py showmigrations 2>&1)" && [ "${CUR_DB_WAIT_TIME}" -lt "${MAX_DB_WAIT_TIME}" ]; do
   echo "⏳ Waiting on DB... (${CUR_DB_WAIT_TIME}s / ${MAX_DB_WAIT_TIME}s)"
   sleep "${DB_WAIT_TIMEOUT}"
   CUR_DB_WAIT_TIME=$((CUR_DB_WAIT_TIME + DB_WAIT_TIMEOUT))
@@ -23,6 +23,33 @@
 if [ "${CUR_DB_WAIT_TIME}" -ge "${MAX_DB_WAIT_TIME}" ]; then
   echo "❌ Waited ${MAX_DB_WAIT_TIME}s or more for the DB to become ready."
   exit 1
+fi
+
+if echo "${SHOWMIGRATIONS}" | grep '^ \[ \] ' >/dev/null; then
+    # Apply any database migrations
+    COMMAND="./manage.py migrate"
+    echo "Applying database migrations ($COMMAND)..."
+    eval $COMMAND || exit 1
+
+    # Trace any missing cable paths (not typically needed)
+    COMMAND="./manage.py trace_paths --no-input"
+    echo "Checking for missing cable paths ($COMMAND)..."
+    eval $COMMAND || exit 1
+
+    # Delete any stale content types
+    COMMAND="./manage.py remove_stale_contenttypes --no-input"
+    echo "Removing stale content types ($COMMAND)..."
+    eval $COMMAND || exit 1
+
+    # Delete any expired user sessions
+    COMMAND="./manage.py clearsessions"
+    echo "Removing expired user sessions ($COMMAND)..."
+    eval $COMMAND || exit 1
+
+    # Clear all cached data
+    COMMAND="./manage.py invalidate all"
+    echo "Clearing cache data ($COMMAND)..."
+    eval $COMMAND || exit 1
 fi # Create Superuser if required

@ryanmerolle
Copy link
Contributor

I think this is an important topic for frequent upgraders. We should devise a way to handle this for sure.

@tobiasge tobiasge mentioned this pull request May 5, 2021
3 tasks
@tobiasge
Copy link
Member

Superseded by PR #507

@tobiasge tobiasge closed this May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue requires further input from the community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants