feat(cloud-init): Implement upcoming cloud-init WSL support #506
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
from
7dbe866 to
7f6e2fd
Compare
EduardGomezEscandell
changed the title
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
from
7f6e2fd to
2371aa0
Compare
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
from
2371aa0 to
47aa0be
Compare
EduardGomezEscandell
changed the base branch from
main
to
landscape-refactor-tests
|
User creation after install is left for a follow-up pull request (#584). |
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
from
d8e434a to
ac7144f
Compare
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
from
ac7144f to
224bcf3
Compare
EduardGomezEscandell
changed the base branch from
landscape-refactor-tests
to
config-notify
EduardGomezEscandell
added a commit
that referenced
this pull request
Jan 30, 2024
This PR concerns the Landscape service package tests that tested the execution of commands comming from the server. It is motivated by the attempt at adding new tests in PR #506 regarding cloud-init integration. These tests where sub-par for two reasons: 1. They were structured in a way that was hard to read 2. They had very little granularity: only one success and one failure test case per command. Adding new tests was quite hard, defeating the purpose of table testing. With this new structure, adding test cases is much simpler, so I added quite a few of them. A side effect of this PR is that the executor tests can no longer be ran without the mock. We were never running them anyways so I decided against maitaining unused code.
EduardGomezEscandell
added a commit
that referenced
this pull request
Feb 1, 2024
I want cloud-init & landscape to be pinged by the config every time
something changes. However, I don't want these two utilities to be
dependencies of config. If anything, I want the dependency the other way
around! Cloud-init and Landscape should import config.
So I must somehow call these utilities from the config without importing
them. My solution: callbacks. Any utility can now wait on config changes
by using:
```go
conf.Notify(func() {
fmt.Println("Hello, world")
})
```
and this function will be called every time the config is changed (via
GUI, registry, or any future methods).
This is used in Landscape #515 and Cloud-init #506.
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
from
224bcf3 to
8e9a840
Compare
EduardGomezEscandell
changed the base branch from
main
to
create-ubuntupro-dir
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
from
8e9a840 to
1bf2eb8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #506 +/- ##
==========================================
+ Coverage 87.24% 87.57% +0.33%
==========================================
Files 97 99 +2
Lines 6224 6344 +120
==========================================
+ Hits 5430 5556 +126
+ Misses 624 623 -1
+ Partials 170 165 -5 ☔ View full report in Codecov by Sentry. |
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
4 times, most recently
from
c947aeb to
231a507
Compare
EduardGomezEscandell
force-pushed
the
fix-config-notify
branch
from
61f1cc7 to
cd4d15f
Compare
EduardGomezEscandell
force-pushed
the
landscape-cloud-init
branch
2 times, most recently
from
565d35f to
f731532
Compare
That function is about agent data, not user's.
it's about cloud-init failing to remove a file, not to write it.
There is no specific reason to split that writing. Also, less lines for codecov to complain about.
Since we are passing nearby the codecov is unhappy
on the testcases definitions we touched on in this PR.
- Removing common golden; - Generating new goldens for each error case -- even if they are equal - Renaming Notify into Update; - Renaming golden directory - to match the new test case name - Asserting on the goldens instead of on err. - There is no tc.wantErr anymore.
It's implied that if New returns an error, the other return value shouldn't be used, yet it's good to return nil so any naive attempt to use it would panic instead of behaving unpredicatably.
What that does even mean? That only returns an error if writeAgentData does so. that could be for a number of errors, all serialization-related. If we returned a usable object under such circunstance and registered it to be notified of changes in the agent's config, silent failures would be logged on update, while the agent would stay ill-running.
And replace it by a want string.
All cases, since cloudinit is now the provisioning method. We still run the end-to-end test workflow to assert that all artifacts can be built.
On the happy path.
We force both closures to run by writing into the registry mock. That mock defers notifying the registry watcher of the changes. The information finally hits cloud-init and we assert on the produced agent.yaml against the goldens herein introduced.
Still need to find a way other than time-based programming to give cloudinit time to write the agent.yaml
It'd be safe today to do so, but might not be tomorrow. The good practice is always to not touch the returned object if err is not nil.
CarlosNihelton
force-pushed
the
landscape-cloud-init
branch
from
32b202a to
d3426c8
Compare
|
Rebased over latest main to solve merge conflicts. Only the last commit is new. |
didrocks
approved these changes
Apr 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cloud-init will, at some point, be able to pick up files from specified locations in the Windows file-system. The plan is for UP4W’s agent to receive cloud-init files from Landscape and put them at the appropriate location.
Furthermore, system-wide config (Pro token and Landscape) must be written as well.
UDENG-2112