Skip to content

stages/systemd.unit.create: fix DefaultDependencies option#608

Closed
say-paul wants to merge 16 commits intoosbuild:mainfrom
say-paul:fix_systemd_defaultdependencies_flag
Closed

stages/systemd.unit.create: fix DefaultDependencies option#608
say-paul wants to merge 16 commits intoosbuild:mainfrom
say-paul:fix_systemd_defaultdependencies_flag

Conversation

@say-paul
Copy link
Copy Markdown
Member

@say-paul say-paul commented Apr 17, 2024 •
edited
Loading

Added a function to pass the bool as pointer.

DefaultDependencies in systemd is by default set as true and needs to be exluscively set as false which is oppositeof golangs syntax omitempty. Leaving out omitempty causes the default dependencies to be set as false by default which will cause the service to behave abnormally as its by default set to true in systemd world.

This PR will fix the behavior

Also refactored the bash scripts for osbuild-ostree-mountpoints.service requird for custom-file-system feature

@say-paul say-paul requested review from mvo5 April 17, 2024 08:29
achilleas-k
achilleas-k requested changes Apr 17, 2024
View reviewed changes
Comment thread pkg/manifest/ostree_deployment.go Outdated
@achilleas-k
Copy link
Copy Markdown
Member

achilleas-k commented Apr 17, 2024

Thanks for spotting this and fixing it.

@say-paul say-paul force-pushed the fix_systemd_defaultdependencies_flag branch from 4c7f966 to e0854a5 Compare April 18, 2024 08:42
@say-paul say-paul requested a review from achilleas-k April 18, 2024 08:47
achilleas-k
achilleas-k previously approved these changes Apr 18, 2024
View reviewed changes
@achilleas-k
Copy link
Copy Markdown
Member

achilleas-k commented Apr 18, 2024

Something's not right with the change in the systemd service's exec line changes because the boot test is showing that service is failing to start now.

@achilleas-k
Copy link
Copy Markdown
Member

achilleas-k commented Apr 18, 2024

I opened #613 that prints failed unit status outputs in the boot test so cases like these are easier to troubleshoot without reproducing by hand.

@achilleas-k achilleas-k force-pushed the fix_systemd_defaultdependencies_flag branch from e0854a5 to 81f6e0b Compare April 23, 2024 17:47
@achilleas-k achilleas-k changed the title stage-systemd-unit-create:fix defaultdependencies flag stages/systemd.unit.create: fix DefaultDependencies option Apr 23, 2024
@achilleas-k achilleas-k self-requested a review April 23, 2024 17:48
@achilleas-k
Copy link
Copy Markdown
Member

achilleas-k commented Apr 24, 2024

@say-paul edge-ami boot tests are failing with:

❌ Status for all failed units
× osbuild-ostree-mountpoints.service - Ensure custom filesystem mountpoints exist
     Loaded: loaded (/etc/systemd/system/osbuild-ostree-mountpoints.service; enabled; preset: enabled)
     Active: failed (Result: exit-code) since Tue 2024-04-23 15:10:35 EDT; 55s ago
    Process: 800 ExecStartPre=/bin/sh -c grep -Uqs composefs /run/ostree-booted || chattr -i / (code=exited, status=0/SUCCESS)
    Process: 807 ExecStart=mkdir -p /foo /foo/bar /root /mnt /srv /opt /var/mydata (code=exited, status=1/FAILURE)
    Process: 808 ExecStopPost=/bin/sh -c grep -Uqs composefs /run/ostree-booted || chattr +i / (code=exited, status=0/SUCCESS)
   Main PID: 807 (code=exited, status=1/FAILURE)
        CPU: 21ms
Apr 23 15:10:35 localhost mkdir[807]: mkdir: cannot create directory ‘/root’: File exists
Apr 23 15:10:35 localhost mkdir[807]: mkdir: cannot create directory ‘/srv’: File exists
Apr 23 15:10:35 localhost mkdir[807]: mkdir: cannot create directory ‘/opt’: File exists
terminating instance i-018c92151576f4400
deleting security group sg-0e5b3f74ac8458cca
deleting EC2 image ami-06894585993d267c6 and snapshot snap-070e520d98107490f
exit status 3
exit status 1

https://gitlab.com/redhat/services/products/image-builder/ci/images/-/jobs/6699182166

@say-paul
Copy link
Copy Markdown
Member Author

say-paul commented Apr 24, 2024

hmmh..!! thought -p would take care of it

@achilleas-k
Copy link
Copy Markdown
Member

achilleas-k commented Apr 24, 2024 •
edited
Loading

Yeah it's strange. I wonder if the change in the way the chattr is handled isn't working properly so the immutable flag isn't being unset. But then I guess the error message would be different.

@achilleas-k
Copy link
Copy Markdown
Member

achilleas-k commented Apr 25, 2024

This will need to be rebased once #632 is merged.

@achilleas-k achilleas-k force-pushed the fix_systemd_defaultdependencies_flag branch from 81f6e0b to 5a74143 Compare April 25, 2024 17:06
say-paul added 2 commits April 29, 2024 20:08
@say-paul @achilleas-k
stage-systemd-unit-create:fix defaultdepencies flag
91ce0df 
Added a function to pass the bool as pointer.

DefaultDependencies in systemd  is by default set as true and needs to be
exluscively set as false which is oppositeof  golangs syntax omitempty.
Leaving out omitempty causes the default dependencies to be set as
false by default which will cause the service to behave abnormally as its
by default set to true in systemd world.

Signed-off-by: Sayan Paul <paul.sayan@gmail.com>
@say-paul @achilleas-k
custom-mountpoints-service:fix the bash script
f328016 
bash command refactoring for osbuild-ostree-mountpoints.service for the
ExecStartPre,ExecStopPost

Signed-off-by: Sayan Paul <paul.sayan@gmail.com>
@achilleas-k achilleas-k force-pushed the fix_systemd_defaultdependencies_flag branch from 5a74143 to f328016 Compare April 29, 2024 18:08
@achilleas-k achilleas-k mentioned this pull request May 7, 2024
Merged
4 tasks
mvo5 and others added 9 commits May 8, 2024 15:29
@mvo5 @say-paul
manifest: ensure /var/roothome is available when needed
4f9f16e 
Currently creating a customization for the root user does not work
because `/var/roothome` may not be avaialble in the image. This
commit ensures it is created if needed just like we do for `/var/home`.

Closes osbuild/bootc-image-builder#143
@dependabot @say-paul
build(deps): bump github.com/aws/aws-sdk-go in the go-deps group
7529414 
Bumps the go-deps group with 1 update: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go).


Updates `github.com/aws/aws-sdk-go` from 1.51.30 to 1.51.31
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.51.30...v1.51.31)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot @say-paul
build(deps): bump the go-deps group with 2 updates
1c4dfe2 
Bumps the go-deps group with 2 updates: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) and [google.golang.org/api](https://github.com/googleapis/google-api-go-client).


Updates `github.com/aws/aws-sdk-go` from 1.51.31 to 1.51.32
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Changelog](https://github.com/aws/aws-sdk-go/blob/main/CHANGELOG_PENDING.md)
- [Commits](aws/aws-sdk-go@v1.51.31...v1.51.32)

Updates `google.golang.org/api` from 0.176.1 to 0.177.0
- [Release notes](https://github.com/googleapis/google-api-go-client/releases)
- [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md)
- [Commits](googleapis/google-api-go-client@v0.176.1...v0.177.0)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot @say-paul
build(deps): bump the go-deps group with 2 updates
a6168a3 
Bumps the go-deps group with 2 updates: [cloud.google.com/go/compute](https://github.com/googleapis/google-cloud-go) and [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go).


Updates `cloud.google.com/go/compute` from 1.25.1 to 1.26.0
- [Release notes](https://github.com/googleapis/google-cloud-go/releases)
- [Changelog](https://github.com/googleapis/google-cloud-go/blob/main/documentai/CHANGES.md)
- [Commits](googleapis/google-cloud-go@pubsub/v1.25.1...pubsub/v1.26.0)

Updates `github.com/aws/aws-sdk-go` from 1.51.32 to 1.52.0
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.51.32...v1.52.0)

---
updated-dependencies:
- dependency-name: cloud.google.com/go/compute
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot @say-paul
build(deps): bump github.com/aws/aws-sdk-go in the go-deps group
e03ae27 
Bumps the go-deps group with 1 update: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go).


Updates `github.com/aws/aws-sdk-go` from 1.52.0 to 1.52.1
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.52.0...v1.52.1)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <support@github.com>
@achilleas-k @say-paul
distro/rhel: unify osname for edge deployments
9379f1f 
Our RHEL Edge deployments (ISOs and disk images) had inconsistent
osnames set.  The edge-installer used "rhel" while the simplified
installer and disk image used "redhat".

For bonus inconsistency, the RHEL Edge ISOs created by the Edge Service
modify the kickstart file to set the osname to "rhel-edge".

Meanwhile, Fedora IoT images all set the osname to "fedora-iot".

To make everything consistent and unify the names, let's set all osnames
to "rhel-edge".  This aligns better with Fedora (the equivalent to
"fedora-iot" in the RHEL world is "rhel-edge") and with what has already
been happening in the service.

Practically, the osname sets the deployment path:

  /ostree/deploy/$OSNAME

so this will affect new deployments, but will have no effect on existing
installations and upgrades.
@achilleas-k @say-paul
manifest: small function for insights-client drop-in
2027db7 
We need to create a drop-in file that will enable
insights-client.service to run on boot for edge systems.

Ref: HMS-4031
@achilleas-k @say-paul
manifest: modify insights-client.service for ostree-based systems
f9434a0 
Insights-client collection must occur at boot time so that the current
ostree commit hash can be reflected after upgrade.  Otherwise, the
upgrade shows as failed in the console UI.  Add a drop-in file that
enables insights-client.service to run on successful boot.

Ref: HMS-4031
@schutzbot @say-paul
schutzfile: Update snapshots to 20240502
8e5c9d5 
- Hold back C9S due to RHEL-29559.
- Hold back RHEL 10 because dhcp-client is not included in the newer
  snapshots (RHEL-26304).
dependabot Bot and others added 3 commits May 8, 2024 15:29
@dependabot @say-paul
build(deps): bump the go-deps group with 3 updates
8afcd5a 
Bumps the go-deps group with 3 updates: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go), [golang.org/x/oauth2](https://github.com/golang/oauth2) and [golang.org/x/sys](https://github.com/golang/sys).


Updates `github.com/aws/aws-sdk-go` from 1.52.1 to 1.52.2
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.52.1...v1.52.2)

Updates `golang.org/x/oauth2` from 0.19.0 to 0.20.0
- [Commits](golang/oauth2@v0.19.0...v0.20.0)

Updates `golang.org/x/sys` from 0.19.0 to 0.20.0
- [Commits](golang/sys@v0.19.0...v0.20.0)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
- dependency-name: golang.org/x/oauth2
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot @say-paul
build(deps): bump github.com/aws/aws-sdk-go in the go-deps group
0039699 
Bumps the go-deps group with 1 update: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go).


Updates `github.com/aws/aws-sdk-go` from 1.52.2 to 1.52.3
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.52.2...v1.52.3)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <support@github.com>
@say-paul
check if file present
207891c 
Signed-off-by: Sayan Paul <paul.sayan@gmail.com>
achilleas-k and others added 2 commits May 8, 2024 15:33
@achilleas-k @say-paul
test/scripts: fail setup-osbuild-repo on bad repo
65ee737 
When testing with "pinned" osbuild commit IDs (defined in the
Schutzfile), we set up a repository based on the commit ID that should
exist for each commit in osbuild that had a (partially) successful CI
run (the RPM jobs should at least run successfully).

When a repository doesn't exist for whatever reason (the CI pipeline
failed, the commit ID was copied incorrectly, etc), the setup script
wouldn't fail and osbuild would be installed from the system
repositories.  This means that we're not testing what we think we are
testing.  In the best case our build jobs fail because the osbuild
version in the repos isn't new enough to have all the features we're
testing, but we might also end up having passing tests that aren't
testing what we want, for example a change in osbuild internals that's
on an open PR and we want to verify that none or our test image builds
are broken.

Make the setup-osbuild-repo script check that the repository metadata is
reachable and exit with an error if it's not, making any test pipeline
that requires it fail when necessary instead of silently skipping the
repo configuration.
@dependabot @say-paul
build(deps): bump the go-deps group with 2 updates
ede9fb5 
Bumps the go-deps group with 2 updates: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) and [google.golang.org/api](https://github.com/googleapis/google-api-go-client).


Updates `github.com/aws/aws-sdk-go` from 1.52.3 to 1.52.4
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.52.3...v1.52.4)

Updates `google.golang.org/api` from 0.177.0 to 0.178.0
- [Release notes](https://github.com/googleapis/google-api-go-client/releases)
- [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md)
- [Commits](googleapis/google-api-go-client@v0.177.0...v0.178.0)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-deps
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-deps
...

Signed-off-by: dependabot[bot] <support@github.com>
@achilleas-k
Copy link
Copy Markdown
Member

achilleas-k commented May 8, 2024

Replaced by #668

@achilleas-k achilleas-k closed this May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mvo5 mvo5 Awaiting requested review from mvo5

@achilleas-k achilleas-k Awaiting requested review from achilleas-k achilleas-k is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@say-paul @achilleas-k @mvo5 @schutzbot