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

publishing collected_sources.tar.gz to eve-sources repo under docker/lfedge #3209

Merged
merged 11 commits into from
May 16, 2023
Merged

Conversation

yash-zededa
Copy link
Collaborator

This change will push the collected_sources artefact created for the SBOM comparison to eve-sources docker repo under lfedge. eve-sources will be a new repo and a ticket is already in place to create the new repo by @eriknordmark

Added a new target publish_sources that depends on collected_sources.

publish_sources will build the docker image from pkg/sources/Dockerfile and push to lfedge/eve-sources docker hub repo

## Usage

```bash
make -e V=1 HV=${{ matrix.hv }} ZARCH=${{ matrix.arch }} LINUXKIT_PKG_TARGET=push publish_sources
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add something about how this does not run locally, but rather is copied to SOURCES_DIR from the Makefile, e.g. dist/amd64/0.0.0-master-..../sources/ and run there, so no one gets confused?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add more info.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added more around the workflow and process

Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like one accidental unrelated change, and a request to add to the README. Other than that, really looks good.

Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We do need to wait for the new lfedge Docker Hub repository, and ensure that it matches what is in build.yaml (if not, then change build.yaml content), but other than that, looks great!

* alpine, kernel and golang packages.
* A manifest file collected_sources_manifest.csv that holds the metadata for all the packages.

### Workflow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yash-zededa can you address these markdownlint complaints?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eriknordmark README.md linting is fixed.

yash-zededa and others added 10 commits May 15, 2023 15:33
Signed-off-by: yash-zededa <yash@zededa.com>
Signed-off-by: yash-zededa <yash@zededa.com>
Signed-off-by: yash-zededa <yash@zededa.com>
Do not activate PhysicalIOAdapterList subscription until NIM receives
a DPC and publishes corresponding DNS.
NIM can publish DNS even before it receives first DPC. In such case
DPCKey is empty.
The goal is to avoid assigning network ports to PCIBack if they are going to be
used for management purposes. This way we avoid doing unintended port assignment
to PCIBack that would be shortly followed by a release, therefore mitigating
the risk of race conditions between domainmgr and NIM.

Signed-off-by: Milan Lenco <milan@zededa.com>
Signed-off-by: yash-zededa <yash@zededa.com>
This is mostly to address error frequently seen when run eden tests on
(slow) github runners:

Rebooting EVE. Reason: Reboot from agent zedbox[1468] in partition IMGA at EVE version pr3184-kvm-amd64 at 2023-05-02T23:53:59.207011689Z: fatal: agent zedbox[1468]: couldn't initialize containerd (this should not happen): initContainerdClient: could not create containerd client. failed to dial "/run/containerd/containerd.sock": context deadline exceeded: failed to receive server preface within timeout. Exiting.

I have not seen this when running tests locally or when running EVE
on baremetal devices.
Note that default timeout for containerd client to connect is 10
seconds. Increasing this to 30 seconds should not have any negative
unintended consequences.

Signed-off-by: Milan Lenco <milan@zededa.com>
Signed-off-by: yash-zededa <yash@zededa.com>
Bring in patches done in the eden repository to address some
unstable eden tests.

Signed-off-by: Milan Lenco <milan@zededa.com>
Signed-off-by: yash-zededa <yash@zededa.com>
The process of converting the declarative high-level network instance
config coming from the controller into the corresponding low-level
(Linux) network stack config items (routes, bridges, iptables, etc.)
has been previously scattered all across the zedrouter codebase and it
was tightly coupled to the Linux network stack. Given the considerable
amount of config items that even a single network instance with only
a couple of ACL rules translates into, the original implementation
ended up being quite complicated to understand and debug/maintain,
not to mention extend. In order to avoid adding even more complexity,
it didn't support config changes (it was necessary to recreate network
instances). Lastly, since the original code would make netlink and other
syscalls directly, without hiding them behind some interface, it was not
possible to "mock" the host system and write some unit tests for zedrouter.

In this commit we introduce a new component NIReconciler, which, just
like NIM's DPCReconciler, is based on the Reconciler (libs/reconciler).
NIReconciler is an interface and LinuxNIReconciler is just one
(and currently the only one) implementation. By using the generic
Reconciler, NIReconciler only needs to determine what the intended
state of the network stack should be for the given EdgeDevConfig
config received from the controller wrt. app connectivity (specifically
it reads the config for network instances and app interfaces).
The problem of intended<->current state reconciliation is taken care of
by the Reconciler, making NIReconciler considerably less complex than
the original zedrouter's approach, while additionally supporting any
config change (that makes sense).

LinuxNIReconciler is covered by unit tests (around 86% coverage,
excluding "mocked" parts).

This commit only introduces NIReconciler as another package under
pillar. It is not yet integrated into zedrouter. This will be done in
the next commit.
The only changes outside of introducing a new package are:
* Small improvements in iptables - e.g. adding support for ipsets
* Split base routing table index between DPC and network instances for
  more clarity
* change HostSubnet to return net.IPNet with pointer, not as a value
  (in net and other standard packages IPNet is always used with pointer)

Signed-off-by: Milan Lenco <milan@zededa.com>
Signed-off-by: yash-zededa <yash@zededa.com>
`EVE_TARGET_ARCH` must be correctly and consistenly set to correclty
build expected targets.  Further, `EVE_BUILD_ARCH` must be set to
reflect the curren build host architecture.  The correct value for
this is provided by `buildkit` as `BUILDARCH`, only used in one
place.

Validated cross build of `arm64`, `amd64`,and  `arm64`.

Signed-off-by: Gerald (Bob) Lee <bob@famleehouse.net>
Signed-off-by: yash-zededa <yash@zededa.com>
Signed-off-by: yash-zededa <yash@zededa.com>
@yash-zededa yash-zededa requested a review from milan-zededa as a code owner May 15, 2023 13:33
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have eve-sources on docker hub so proceeding. Let's see how the assets workflow works.

@eriknordmark eriknordmark merged commit 26e75df into lf-edge:master May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants