Skip to content

Commit

Permalink
Use nmstatectl rust (nmstate#1059)
Browse files Browse the repository at this point in the history
* build: Base containers on centos stream 9

Centos stream 9 nmstatectl is rust based. This change use centos stream
9 both at main and future.

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* nnce: Don't strip nmstate error

The nmstatectl rust has a much more concise error when fail, there is no
stack trace. This change remove the error stripping and their tests.

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* nns: Filter out type "ignore"

The rust implementation set unmanaged interfaces as "ignore"

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* cluster: Workaround for bz#2089791

Looks like upgrading NetworkManager for 1.37 to 1.39 fix the issue
reported at https://bugzilla.redhat.com/show_bug.cgi?id=2089791

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* test: Don't check guest libnm version

Nmstate no longer uses libnm so the libnm version is not needed.

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* handler: Remove plugin-ovsdb

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* test: Print stderr on Run error

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* test: Retrieve initial state at BeforeSuite

In case on test fail to reset state next test will fail comparing with
invalid initial state. This change retrieve the initial state before run
the tests so the compare with pre tests values.

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* handler: Install nmcli

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* node: Fix versions retrieval errors

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* kubevirtci: Bump to 1.26 c9s

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* handler: Pin nmstate to #2280

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* kubevirtci: Update KUBEVIRTCI_TAG

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

* test, e2e: Use proper apply time

Signed-off-by: Enrique Llorente <ellorent@redhat.com>

---------

Signed-off-by: Enrique Llorente <ellorent@redhat.com>
  • Loading branch information
qinqon committed Mar 23, 2023
1 parent 38d7792 commit 5bfb437
Show file tree
Hide file tree
Showing 29 changed files with 133 additions and 774 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ IMAGE_REPO ?= nmstate
NAMESPACE ?= nmstate

ifeq ($(NMSTATE_PIN), future)
HANDLER_EXTRA_PARAMS:= "--build-arg NMSTATE_SOURCE=git --build-arg FROM=quay.io/centos/centos:stream9"
HANDLER_EXTRA_PARAMS:= "--build-arg NMSTATE_SOURCE=git"
endif

HANDLER_IMAGE_NAME ?= kubernetes-nmstate-handler
Expand All @@ -32,7 +32,7 @@ WHAT ?= ./pkg/... ./controllers/...

unit_test_args ?= -r -keep-going --randomize-all --randomize-suites --race --trace $(UNIT_TEST_ARGS)

export KUBEVIRT_PROVIDER ?= k8s-1.24
export KUBEVIRT_PROVIDER ?= k8s-1.26-centos9
export KUBEVIRT_NUM_NODES ?= 2 # 1 control-plane, 1 worker needed for e2e tests
export KUBEVIRT_NUM_SECONDARY_NICS ?= 2

Expand Down
1 change: 0 additions & 1 deletion api/shared/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ type Condition struct {
Status corev1.ConditionStatus `json:"status"`
Reason ConditionReason `json:"reason,omitempty"`
Message string `json:"message,omitempty"`
MessageEncoded string `json:"messageEncoded,omitempty"`
LastHeartbeatTime metav1.Time `json:"lastHeartbeatTime,omitempty"`
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
}
Expand Down
6 changes: 3 additions & 3 deletions build/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ARG FROM=quay.io/centos/centos:stream8
ARG FROM=quay.io/centos/centos:stream9
FROM --platform=linux/amd64 ${FROM} AS build

ARG TARGETARCH
Expand All @@ -14,7 +14,7 @@ COPY . .

RUN --mount=type=cache,target=/root/.cache/go-build GOOS=linux GOARCH=${TARGETARCH} go build -o /manager ./cmd/handler

ARG FROM=quay.io/centos/centos:stream8
ARG FROM=quay.io/centos/centos:stream9
FROM ${FROM}

ARG NMSTATE_SOURCE=distro
Expand All @@ -23,7 +23,7 @@ COPY --from=build /manager /usr/local/bin/manager
COPY --from=build /workdir/build/install-nmstate.${NMSTATE_SOURCE}.sh install-nmstate.sh

RUN ./install-nmstate.sh && \
dnf install -b -y iproute iputils && \
dnf install -b -y NetworkManager iproute iputils && \
rm ./install-nmstate.sh && \
dnf clean all

Expand Down
6 changes: 5 additions & 1 deletion build/install-nmstate.distro.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
#!/bin/bash -xe

dnf install -b -y -x "*alpha*" -x "*beta*" nmstate nmstate-plugin-ovsdb
#dnf install -b -y -x "*alpha*" -x "*beta*" nmstate

dnf install -b -y dnf-plugins-core
dnf copr enable -y packit/nmstate-nmstate-2280
dnf install -b -y nmstate
2 changes: 1 addition & 1 deletion build/install-nmstate.git.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

dnf install -b -y dnf-plugins-core
dnf copr enable -y nmstate/nmstate-git
dnf install -b -y nmstate nmstate-plugin-ovsdb
dnf install -b -y nmstate
4 changes: 0 additions & 4 deletions bundle/manifests/nmstate.io_nmstates.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1812,8 +1812,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -1970,8 +1968,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -170,8 +168,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -187,8 +185,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -283,8 +279,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
4 changes: 0 additions & 4 deletions bundle/manifests/nmstate.io_nodenetworkstates.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -112,8 +110,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
4 changes: 2 additions & 2 deletions cluster/kubevirtci.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export KUBEVIRT_PROVIDER=${KUBEVIRT_PROVIDER:-'k8s-1.24'}
export KUBEVIRTCI_TAG=2211021552-8cca8c0
export KUBEVIRT_PROVIDER=${KUBEVIRT_PROVIDER:-'k8s-1.26-centos9'}
export KUBEVIRTCI_TAG=2303201102-ef46217

KUBEVIRTCI_REPO='https://github.com/kubevirt/kubevirtci.git'
KUBEVIRTCI_PATH="${PWD}/_kubevirtci"
Expand Down
3 changes: 2 additions & 1 deletion cluster/up.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ for node in $(./cluster/kubectl.sh get nodes --no-headers | awk '{print $1}'); d
done
done

echo 'Enabling and starting up openvswitch'
echo 'Upgrading NetworkManager and enabling and starting up openvswitch'
for node in $(./cluster/kubectl.sh get nodes --no-headers | awk '{print $1}'); do
./cluster/cli.sh ssh ${node} -- sudo dnf upgrade -y NetworkManager
./cluster/cli.sh ssh ${node} -- sudo systemctl daemon-reload
./cluster/cli.sh ssh ${node} -- sudo systemctl enable openvswitch
./cluster/cli.sh ssh ${node} -- sudo systemctl restart openvswitch
Expand Down
10 changes: 4 additions & 6 deletions controllers/handler/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ type NodeReconciler struct {
lastState shared.State
nmstateUpdater NmstateUpdater
nmstatectlShow NmstatectlShow
deviceInfo state.DeviceInfoer
}

// Reconcile reads that state of the cluster for a Node object and makes changes based on the state read
Expand All @@ -77,7 +76,7 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, request ctrl.Request) (c
return ctrl.Result{}, err
}

currentState, err := state.FilterOut(shared.NewState(currentStateRaw), r.deviceInfo)
currentState, err := state.FilterOut(shared.NewState(currentStateRaw))
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -126,22 +125,22 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, request ctrl.Request) (c
func (r *NodeReconciler) getDependencyVersions() *nmstate.DependencyVersions {
handlerNetworkManagerVersion, err := nmstate.ExecuteCommand("nmcli", "--version")
if err != nil {
r.Log.Info("error retrieving handler NetworkManager version: %s", err.Error())
r.Log.Error(err, "failed retrieving handler NetworkManager version")
}
// remove leading characters up to last space
split := strings.Split(handlerNetworkManagerVersion, " ")
handlerNetworkManagerVersion = split[len(split)-1]

handlerNmstateVersion, err := nmstate.ExecuteCommand("nmstatectl", "--version")
if err != nil {
r.Log.Info("error retrieving handler nmstate version: %s", err.Error())
r.Log.Error(err, "failed retrieving handler nmstate version")
}

hostNmstateVersion := ""
nmClient, err := networkmanager.NewClientPrivate()

if err != nil {
r.Log.Info("error retrieving new client: %s", err.Error())
r.Log.Error(err, "failed retrieving new client")

return &nmstate.DependencyVersions{
HandlerNetworkManagerVersion: handlerNetworkManagerVersion,
Expand All @@ -167,7 +166,6 @@ func (r *NodeReconciler) getDependencyVersions() *nmstate.DependencyVersions {
func (r *NodeReconciler) SetupWithManager(mgr ctrl.Manager) error {
r.nmstateUpdater = nmstate.CreateOrUpdateNodeNetworkState
r.nmstatectlShow = nmstatectl.Show
r.deviceInfo = state.DeviceInfo{}

// By default all this functors return true so controller watch all events,
// but we only want to watch create/delete for current node.
Expand Down
16 changes: 2 additions & 14 deletions controllers/handler/node_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

networkmanager "github.com/phoracek/networkmanager-go/src"

"github.com/nmstate/kubernetes-nmstate/api/shared"
nmstatev1beta1 "github.com/nmstate/kubernetes-nmstate/api/v1beta1"
"github.com/nmstate/kubernetes-nmstate/pkg/nmstatectl"
nmstatenode "github.com/nmstate/kubernetes-nmstate/pkg/node"
"github.com/nmstate/kubernetes-nmstate/pkg/state"
)

type fakeDeviceInfo struct{}

func (fakeDeviceInfo) DeviceStates() (map[string]networkmanager.DeviceState, error) {
return map[string]networkmanager.DeviceState{
"eth1": networkmanager.DeviceStateActivated,
"eth2": networkmanager.DeviceStateActivated,
}, nil
}

var _ = Describe("Node controller reconcile", func() {
var (
cl client.Client
Expand Down Expand Up @@ -100,7 +89,6 @@ var _ = Describe("Node controller reconcile", func() {
reconciler.Scheme = s
reconciler.nmstateUpdater = nmstate.CreateOrUpdateNodeNetworkState
reconciler.nmstatectlShow = nmstatectl.Show
reconciler.deviceInfo = fakeDeviceInfo{}
reconciler.lastState = shared.NewState("lastState")
observedState = `
---
Expand All @@ -114,7 +102,7 @@ routes:
`

var err error
filteredOutObservedState, err = state.FilterOut(shared.NewState(observedState), reconciler.deviceInfo)
filteredOutObservedState, err = state.FilterOut(shared.NewState(observedState))
Expect(err).ToNot(HaveOccurred())

reconciler.nmstatectlShow = func() (string, error) {
Expand Down Expand Up @@ -209,7 +197,7 @@ routes:
obtainedNNS := nmstatev1beta1.NodeNetworkState{}
err = cl.Get(context.TODO(), types.NamespacedName{Name: existingNodeName}, &obtainedNNS)
Expect(err).ToNot(HaveOccurred())
filteredOutExpectedState, err := state.FilterOut(shared.NewState(expectedStateRaw), reconciler.deviceInfo)
filteredOutExpectedState, err := state.FilterOut(shared.NewState(expectedStateRaw))
Expect(err).ToNot(HaveOccurred())
Expect(obtainedNNS.Status.CurrentState.String()).To(Equal(filteredOutExpectedState.String()))
})
Expand Down
4 changes: 0 additions & 4 deletions deploy/crds/nmstate.io_nmstates.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1813,8 +1813,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -1971,8 +1969,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -171,8 +169,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
6 changes: 0 additions & 6 deletions deploy/crds/nmstate.io_nodenetworkconfigurationpolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -188,8 +186,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -284,8 +280,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
4 changes: 0 additions & 4 deletions deploy/crds/nmstate.io_nodenetworkstates.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down Expand Up @@ -113,8 +111,6 @@ spec:
type: string
message:
type: string
messageEncoded:
type: string
reason:
type: string
status:
Expand Down
9 changes: 1 addition & 8 deletions pkg/enactmentstatus/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,7 @@ func (ec *EnactmentConditions) updateEnactmentConditions(
}

func SetFailedToConfigure(conditions *nmstate.ConditionList, message string) {
strippedMessage := enactmentstatus.FormatErrorString(message)
SetFailed(conditions, nmstate.NodeNetworkConfigurationEnactmentConditionFailedToConfigure, strippedMessage)
setFailedToConfigureEncodedMessage(conditions, message)
}

func setFailedToConfigureEncodedMessage(conditions *nmstate.ConditionList, message string) {
condition := conditions.Find(nmstate.NodeNetworkConfigurationEnactmentConditionFailing)
condition.MessageEncoded = enactmentstatus.CompressAndEncodeMessage(message)
SetFailed(conditions, nmstate.NodeNetworkConfigurationEnactmentConditionFailedToConfigure, message)
}

func SetFailed(conditions *nmstate.ConditionList, reason nmstate.ConditionReason, message string) {
Expand Down
Loading

0 comments on commit 5bfb437

Please sign in to comment.