Skip to content

Commit

Permalink
Issue #615 - fixed readiness-gate Git conflicts
Browse files Browse the repository at this point in the history
- previous solution broke existing functionality to copy comments between Kptfile objects
  - resulting in failing unit tests
    - fix for that brought back the Git conflicts caused by indentation, field order etc.
    - re-refactored conversion of Kptfiles to YAML
      - KubeObject-related conversion methods (KubeObject/YAML and YAML/KubeObject)
        now live out in the util package as they aren't Kptfile-specific any more

nephio-project/nephio#615
  • Loading branch information
JamesMcDermott committed Dec 12, 2024
1 parent bd38163 commit ec61e77
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 67 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2023 The kpt and Nephio Authors
// Copyright 2023-2024 The kpt and Nephio Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -27,6 +27,7 @@ import (
porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
api "github.com/nephio-project/porch/controllers/packagevariants/api/v1alpha1"
kptfilev1 "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1"
"github.com/nephio-project/porch/pkg/util"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -122,11 +123,7 @@ func ensureConfigInjection(ctx context.Context,

setInjectionPointConditionsAndGates(kptfile, injectionPoints)

kptfileYaml, err := kptfilev1.ToYamlString(kptfile)
if err != nil {
return err
}
prr.Spec.Resources[kptfilev1.KptFileName] = kptfileYaml
prr.Spec.Resources[kptfilev1.KptFileName] = util.KubeObjectToYaml(kptfile)

return nil
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022 The kpt and Nephio Authors
// Copyright 2022, 2024 The kpt and Nephio Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -28,6 +28,7 @@ import (
"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
kptfilev1 "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1"
"github.com/nephio-project/porch/pkg/kpt/kptfileutil"
"github.com/nephio-project/porch/pkg/util"

"golang.org/x/mod/semver"
"k8s.io/apimachinery/pkg/api/meta"
Expand Down Expand Up @@ -1080,11 +1081,7 @@ func ensureKRMFunctions(pv *api.PackageVariant,
}

// update kptfile
kptfileYaml, err := kptfilev1.ToYamlString(kptfile)
if err != nil {
return err
}
prr.Spec.Resources[kptfilev1.KptFileName] = kptfileYaml
prr.Spec.Resources[kptfilev1.KptFileName] = util.KubeObjectToYaml(kptfile)

return nil
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2023 The kpt and Nephio Authors
# Copyright 2023-2024 The kpt and Nephio Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -28,7 +28,7 @@ spec:
- foo-01
- foo-02
- foo-03
name: cluster-02
- name: cluster-02
template:
downstream:
packageExpr: "target.package + '-' + repository.labels['env']"
Expand Down
4 changes: 2 additions & 2 deletions func/internal/podevaluator.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022 The kpt and Nephio Authors
// Copyright 2022, 2024 The kpt and Nephio Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -238,7 +238,7 @@ type imagePodAndGRPCClient struct {
func (pcm *podCacheManager) warmupCache(podTTLConfig string) error {
start := time.Now()
defer func() {
klog.Infof("cache warning is completed and it took %v", time.Since(start))
klog.Infof("cache warming is complete after %v", time.Since(start))
}()
content, err := os.ReadFile(podTTLConfig)
if err != nil {
Expand Down
32 changes: 15 additions & 17 deletions pkg/kpt/api/kptfile/v1/types.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2021 The kpt and Nephio Authors
// Copyright 2021, 2024 The kpt and Nephio Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -21,6 +21,7 @@ import (
"fmt"

"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
"github.com/nephio-project/porch/pkg/util"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/kustomize/kyaml/yaml"
)
Expand Down Expand Up @@ -77,13 +78,13 @@ type KptFile struct {
// Info contains metadata such as license, documentation, etc.
Info *PackageInfo `yaml:"info,omitempty" json:"info,omitempty"`

Status *Status `yaml:"status,omitempty" json:"status,omitempty"`

// Pipeline declares the pipeline of functions.
Pipeline *Pipeline `yaml:"pipeline,omitempty" json:"pipeline,omitempty"`

// Inventory contains parameters for the inventory object used in apply.
Inventory *Inventory `yaml:"inventory,omitempty" json:"inventory,omitempty"`

Status *Status `yaml:"status,omitempty" json:"status,omitempty"`
}

func FromKubeObject(kptfileKubeObject *fn.KubeObject) (KptFile, error) {
Expand All @@ -99,16 +100,12 @@ func (file *KptFile) ToYamlString() (string, error) {
if err != nil {
return "", err
}
return string(b), nil
}

func ToYamlString(kptfileKubeObject *fn.KubeObject) (string, error) {
kptfile, err := FromKubeObject(kptfileKubeObject)
kptfileKubeObject, err := util.YamlToKubeObject(string(b))
if err != nil {
return "", err
}

return kptfile.ToYamlString()
return kptfileKubeObject.String(), nil
}

// OriginType defines the type of origin for a package.
Expand Down Expand Up @@ -236,6 +233,8 @@ type PackageInfo struct {
// Relative slash-delimited path to the license file (e.g. LICENSE.txt)
LicenseFile string `yaml:"licenseFile,omitempty" json:"licenseFile,omitempty"`

ReadinessGates []ReadinessGate `yaml:"readinessGates,omitempty" json:"readinessGates,omitempty"`

// Description contains a short description of the package.
Description string `yaml:"description,omitempty" json:"description,omitempty"`

Expand All @@ -244,8 +243,6 @@ type PackageInfo struct {

// Man is the path to documentation about the package
Man string `yaml:"man,omitempty" json:"man,omitempty"`

ReadinessGates []ReadinessGate `yaml:"readinessGates,omitempty" json:"readinessGates,omitempty"`
}

type ReadinessGate struct {
Expand Down Expand Up @@ -315,6 +312,11 @@ func (p *Pipeline) IsEmpty() bool {
// Function specifies a KRM function.
// +kubebuilder:object:generate=true
type Function struct {

// `Name` is used to uniquely identify the function declaration
// this is primarily used for merging function declaration with upstream counterparts
Name string `yaml:"name,omitempty" json:"name,omitempty"`

// `Image` specifies the function container image.
// It can either be fully qualified, e.g.:
//
Expand Down Expand Up @@ -344,10 +346,6 @@ type Function struct {
// `ConfigMap` is a convenient way to specify a function config of kind ConfigMap.
ConfigMap map[string]string `yaml:"configMap,omitempty" json:"configMap,omitempty"`

// `Name` is used to uniquely identify the function declaration
// this is primarily used for merging function declaration with upstream counterparts
Name string `yaml:"name,omitempty" json:"name,omitempty"`

// `Selectors` are used to specify resources on which the function should be executed
// if not specified, all resources are selected
Selectors []Selector `yaml:"selectors,omitempty" json:"selectors,omitempty"`
Expand Down Expand Up @@ -412,9 +410,9 @@ type Condition struct {

Status ConditionStatus `yaml:"status" json:"status"`

Reason string `yaml:"reason,omitempty" json:"reason,omitempty"`

Message string `yaml:"message,omitempty" json:"message,omitempty"`

Reason string `yaml:"reason,omitempty" json:"reason,omitempty"`
}

type ConditionStatus string
Expand Down
32 changes: 16 additions & 16 deletions pkg/kpt/kptfileutil/util_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 The kpt and Nephio Authors
// Copyright 2020, 2024 The kpt and Nephio Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -364,8 +364,8 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
`,
},
"additional readinessGate and condition added in upstream": {
Expand All @@ -381,8 +381,8 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
`,
updated: `
apiVersion: kpt.dev/v1
Expand All @@ -397,12 +397,12 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
- type: bar
status: "False"
reason: reason
message: message
reason: reason
`,
local: `
apiVersion: kpt.dev/v1
Expand All @@ -416,8 +416,8 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
`,
updateUpstream: false,
expected: `
Expand All @@ -433,12 +433,12 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
- type: bar
status: "False"
reason: reason
message: message
reason: reason
`,
},
"readinessGate added removed in upstream": {
Expand Down Expand Up @@ -502,12 +502,12 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
- type: bar
status: "False"
reason: reason
message: message
reason: reason
`,
updated: `
apiVersion: kpt.dev/v1
Expand All @@ -522,12 +522,12 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
- type: zork
status: "Unknown"
reason: reason
message: message
reason: reason
`,
local: `
apiVersion: kpt.dev/v1
Expand All @@ -542,12 +542,12 @@ status:
conditions:
- type: xandar
status: "True"
reason: reason
message: message
reason: reason
- type: foo
status: "True"
reason: reason
message: message
reason: reason
`,
updateUpstream: false,
expected: `
Expand All @@ -563,12 +563,12 @@ status:
conditions:
- type: foo
status: "True"
reason: reason
message: message
reason: reason
- type: zork
status: Unknown
reason: reason
message: message
reason: reason
`,
},
}
Expand Down
12 changes: 0 additions & 12 deletions pkg/task/generictaskhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"fmt"

sdkfn "github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
api "github.com/nephio-project/porch/api/porch/v1alpha1"
configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
"github.com/nephio-project/porch/internal/kpt/builtins"
Expand Down Expand Up @@ -525,17 +524,6 @@ func healConfig(old, new map[string]string) (map[string]string, error) {

healed := out.output.Contents

var kptfileKubeObject *sdkfn.KubeObject
if kptfileKubeObject, err = sdkfn.ParseKubeObject([]byte(healed[kptfile.KptFileName])); err != nil {
return nil, err
}

kptfileYaml, err := kptfile.ToYamlString(kptfileKubeObject)
if err != nil {
return nil, err
}
healed[kptfile.KptFileName] = kptfileYaml

for k, v := range extra {
healed[k] = v
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/task/kio.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ func (r *packageReader) Read() ([]*yaml.RNode, error) {
kioutil.PathAnnotation: k,
},
DisableUnwrapping: true,
// need to preserve indentation to avoid Git conflicts
// in written-out YAML
PreserveSeqIndent: true,
}
nodes, err := reader.Read()
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/task/patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"github.com/nephio-project/porch/pkg/repository/fake"
)

func TestSomething(t *testing.T) {
func TestGenerateKptfilePatches(t *testing.T) {
testCases := map[string]struct {
repoPkgRev repository.PackageRevision
newApiPkgRev *api.PackageRevision
Expand Down Expand Up @@ -145,12 +145,12 @@ func TestSomething(t *testing.T) {
conditions:
- type: foo
status: "True"
+ reason: reason
+ message: message
+ reason: reason
+ - type: bar
+ status: "False"
+ reason: reason
+ message: message
+ reason: reason
`) + "\n",
PatchType: api.PatchTypePatchFile,
},
Expand Down Expand Up @@ -279,12 +279,12 @@ func TestSomething(t *testing.T) {
conditions:
- type: foo
status: "True"
- reason: reason
- message: message
- reason: reason
- - type: bar
- status: "False"
- reason: reason
- message: message
- reason: reason
`) + "\n",
PatchType: api.PatchTypePatchFile,
},
Expand Down
Loading

0 comments on commit ec61e77

Please sign in to comment.