-
Notifications
You must be signed in to change notification settings - Fork 51
Updated the StatefulSet second exercise #781
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
base: master
Are you sure you want to change the base?
Conversation
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.
Other comments (6)
-
examples/ch9/statefulsets/counterapp/server.js (39-41)
Similar to the previous comment, errors in `writeCounter()` should be logged:
} catch (e) { console.error('Error writing counter:', e); return false; } - examples/ch9/statefulsets/counterapp/server.js (18-33) The application uses synchronous file operations (`fs.existsSync`, `fs.mkdirSync`, `fs.writeFileSync`, `fs.readFileSync`) which can block the Node.js event loop. For a production application, consider using the asynchronous versions of these functions to improve performance under load.
-
examples/ch9/statefulsets/counterapp/server.js (27-29)
The `JSON.parse()` call could throw an exception if the file contains invalid JSON, but it's not specifically handled. Consider adding a try-catch specifically for the parsing operation:
const raw = fs.readFileSync(DATA_FILE, 'utf8'); try { const obj = JSON.parse(raw); return typeof obj.count === 'number' ? obj.count : 0; } catch (parseError) { console.error('Error parsing counter file:', parseError); return 0; } -
examples/ch9/statefulsets/counterapp/src/app.js (1-7)
The fetchState function should provide error feedback to the user when the API call fails. Consider adding error handling to display a message when the API is unavailable.
async function fetchState() { try { const res = await fetch('/api/state'); if (!res.ok) { document.getElementById('count').textContent = 'Error'; document.getElementById('podName').textContent = 'API unavailable'; return; } const data = await res.json(); document.getElementById('count').textContent = data.count; document.getElementById('podName').textContent = `pod: ${data.podName}`; } catch (error) { document.getElementById('count').textContent = 'Error'; document.getElementById('podName').textContent = 'API unavailable'; console.error('Failed to fetch state:', error); } } -
examples/ch9/statefulsets/counterapp/src/app.js (9-14)
The increment function should provide error feedback when the API call fails. Consider adding error handling similar to fetchState.
async function increment() { try { const res = await fetch('/api/increment', { method: 'POST' }); if (!res.ok) { document.getElementById('count').textContent = 'Error'; return; } const data = await res.json(); document.getElementById('count').textContent = data.count; } catch (error) { document.getElementById('count').textContent = 'Error'; console.error('Failed to increment count:', error); } } -
examples/ch9/statefulsets/counterapp/src/app.js (16-19)
Consider adding a periodic refresh to automatically update the counter state. This would be particularly useful in a StatefulSet demonstration to show real-time updates from other pods.
window.addEventListener('DOMContentLoaded', () => { document.getElementById('incrementBtn').addEventListener('click', increment); fetchState(); // Refresh state every 5 seconds setInterval(fetchState, 5000); });
💡 To request another review, post a new comment with "/windsurf-review".
| 2. Copy the contents of `examples/ch9/volumes/random-num-pod.yaml` into a new file called `examples/ch9/statefulsets/random-num-statefulset.yaml` and modify it to use a `StatefulSet` resource instead of a `Pod` resource. Set the number of replicas to 3. | ||
|
|
||
| > To prevent the container from dying and restarting, add a sleep command to the container: | ||
| `args: ["shuf -i 0-100 -n 1 >> /opt/number.out; sleep 10000"]`. |
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.
The shell command in the args array needs to be properly quoted to be executed as a single command:
| `args: ["shuf -i 0-100 -n 1 >> /opt/number.out; sleep 10000"]`. | |
| `args: ["/bin/sh", "-c", "shuf -i 0-100 -n 1 >> /opt/number.out; sleep 10000"]`. |
| 1. Destroy and recreate a new cluster. | ||
| `unset KUBECONFIG; k3d cluster delete <cluster-name>; k3d cluster create <cluster-name>`. | ||
|
|
||
| 2. Create a `StatefulSet` for the counterapp. |
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.
The exercise should explicitly instruct students to configure persistent storage for the counter app StatefulSet using volumeClaimTemplates. Without this, the counter state won't persist when pods are deleted, which is a key learning objective for StatefulSets.
| } catch (e) { | ||
| return 0; | ||
| } |
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.
The catch blocks in both readCounter() and writeCounter() functions silently swallow errors. Consider logging the actual error message to help with debugging:
| } catch (e) { | |
| return 0; | |
| } | |
| } catch (e) { | |
| console.error('Error reading counter:', e); | |
| return 0; | |
| } |
The second exercise provides a Dockerfile + code for a simple web app, the rest of the implementation is up to the user.
As opposed to what I originally had (Helm chart and some k8s manifests provided).