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.

Sign up for GitHub

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

Jump to bottom

Adding support for daemonSet and statefulSet resources in charts #405

Merged
merged 6 commits into from
Sep 1, 2023
Merged

Adding support for daemonSet and statefulSet resources in charts #405

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2ae34b8
Adding support for daemonSet and statefulSet resources in charts
Aug 10, 2023
2af321c
Fixing formatting
Aug 16, 2023
380f7c2
adding comments and cleaning up functions
Aug 17, 2023
50701f3
re-run tests
Aug 17, 2023
09fc7af
refactoring WaitForWorkloadResources and adding comments
Aug 29, 2023
562919e
fixing test errors
Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/chartverifier/checks/charttesting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func testRelease(
release, namespace, releaseSelector string,
cleanupHelmTests bool,
) error {
if err := kubectl.WaitForDeployments(ctx, namespace, releaseSelector); err != nil {
if err := kubectl.WaitForWorkloadResources(ctx, namespace, releaseSelector); err != nil {
return err
}
if err := helm.Test(ctx, namespace, release); err != nil {
Expand Down
100 changes: 73 additions & 27 deletions internal/tool/kubectl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ var content embed.FS
var (
kubeOpenShiftVersionMap map[string]string
listDeployments = getDeploymentsList
listDaemonSets = getDaemonSetsList
listStatefulSets = getStatefulSetsList
latestKubeVersion *semver.Version
)

Expand All @@ -41,9 +43,10 @@ type versionMapping struct {
OcpVersion string `yaml:"ocp-version"`
}

type deploymentNotReady struct {
Name string
Unavailable int32
type workloadNotReady struct {
ResourceType string
Name string
Unavailable int32
}

func init() {
Expand Down Expand Up @@ -94,49 +97,76 @@ func NewKubectl(kubeConfig clientcmd.ClientConfig) (*Kubectl, error) {
return kubectl, nil
}

func (k Kubectl) WaitForDeployments(context context.Context, namespace string, selector string) error {
func (k Kubectl) WaitForWorkloadResources(context context.Context, namespace string, selector string) error {
mgoerens marked this conversation as resolved.
Show resolved Hide resolved
deadline, _ := context.Deadline()
unavailableDeployments := []deploymentNotReady{{Name: "none", Unavailable: 1}}
getDeploymentsError := ""

utils.LogInfo(fmt.Sprintf("Start wait for deployments. --timeout time left: %s ", time.Until(deadline).String()))

for deadline.After(time.Now()) && len(unavailableDeployments) > 0 {
unavailableDeployments = []deploymentNotReady{}
deployments, err := listDeployments(k, context, namespace, selector)
if err != nil {
unavailableDeployments = []deploymentNotReady{{Name: "none", Unavailable: 1}}
getDeploymentsError = fmt.Sprintf("error getting deployments from namespace %s : %v", namespace, err)
utils.LogWarning(getDeploymentsError)
unavailableWorkloadResources := []workloadNotReady{{Name: "none", Unavailable: 1}}

getWorkloadResourceError := ""

utils.LogInfo(fmt.Sprintf("Start wait for workloads resources. --timeout time left: %s ", time.Until(deadline).String()))
for deadline.After(time.Now()) && len(unavailableWorkloadResources) > 0 {
unavailableWorkloadResources = []workloadNotReady{}

deployments, errDeployment := listDeployments(k, context, namespace, selector)
daemonSets, errDaemonSets := listDaemonSets(k, context, namespace, selector)
statefulSets, errStatefulSets := listStatefulSets(k, context, namespace, selector)

if errDeployment != nil {
unavailableWorkloadResources = []workloadNotReady{{Name: "none", ResourceType: "Deployment", Unavailable: 1}}
getWorkloadResourceError = fmt.Sprintf("error getting deployments from namespace %s : %v", namespace, errDeployment)
utils.LogWarning(getWorkloadResourceError)
time.Sleep(time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of sleeping here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was actually left over from what the function previously did.
I believe the idea is to avoid excessive calls and rate limiting since it'll immediately hit the API again after these failures. Happy to remove it if this is a non-issue but trying to find the default rate limit info first

} else if errDaemonSets != nil {
unavailableWorkloadResources = []workloadNotReady{{Name: "none", ResourceType: "DaemonSet", Unavailable: 1}}
getWorkloadResourceError = fmt.Sprintf("error getting daemon sets from namespace %s : %v", namespace, errDaemonSets)
utils.LogWarning(getWorkloadResourceError)
time.Sleep(time.Second)
} else if errStatefulSets != nil {
unavailableWorkloadResources = []workloadNotReady{{Name: "none", ResourceType: "StatefulSet", Unavailable: 1}}
getWorkloadResourceError = fmt.Sprintf("error getting stateful sets from namespace %s : %v", namespace, errStatefulSets)
utils.LogWarning(getWorkloadResourceError)
time.Sleep(time.Second)
} else {
getDeploymentsError = ""
getWorkloadResourceError = ""
for _, deployment := range deployments {
// Just after rollout, pods from the previous deployment revision may still be in a
// terminating state.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really get this comment. Here we check if the pods have become available, and I don't really what this has to do with a previous deployment. Wdyt ? Should we remove/update it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call. I'll double check all the comments and make them more relevant.

if deployment.Status.UnavailableReplicas > 0 {
unavailableDeployments = append(unavailableDeployments, deploymentNotReady{Name: deployment.Name, Unavailable: deployment.Status.UnavailableReplicas})
unavailableWorkloadResources = append(unavailableWorkloadResources, workloadNotReady{Name: deployment.Name, ResourceType: "Deployment", Unavailable: deployment.Status.UnavailableReplicas})
}
}
if len(unavailableDeployments) > 0 {
utils.LogInfo(fmt.Sprintf("Wait for %d deployments:", len(unavailableDeployments)))
for _, unavailableDeployment := range unavailableDeployments {
utils.LogInfo(fmt.Sprintf(" - %s with %d unavailable replicas", unavailableDeployment.Name, unavailableDeployment.Unavailable))
for _, daemonSet := range daemonSets {
if daemonSet.Status.NumberUnavailable > 0 {
unavailableWorkloadResources = append(unavailableWorkloadResources, workloadNotReady{Name: daemonSet.Name, ResourceType: "DaemonSet", Unavailable: daemonSet.Status.NumberUnavailable})
}
}
for _, statefulSet := range statefulSets {
// TODO: Double check that Replicas is requested and Available is currently running
unavailableReplicas := statefulSet.Status.Replicas - statefulSet.Status.AvailableReplicas
if unavailableReplicas > 0 {
unavailableWorkloadResources = append(unavailableWorkloadResources, workloadNotReady{Name: statefulSet.Name, ResourceType: "StatefulSet", Unavailable: unavailableReplicas})
}
}

if len(unavailableWorkloadResources) > 0 {
utils.LogInfo(fmt.Sprintf("Wait for %d workload resources:", len(unavailableWorkloadResources)))
for _, unavailableWorkloadResource := range unavailableWorkloadResources {
utils.LogInfo(fmt.Sprintf(" - %s %s with %d unavailable pods", unavailableWorkloadResource.ResourceType, unavailableWorkloadResource.Name, unavailableWorkloadResource.Unavailable))
}
time.Sleep(time.Second)
} else {
utils.LogInfo(fmt.Sprintf("Finish wait for deployments, --timeout time left %s", time.Until(deadline).String()))
utils.LogInfo(fmt.Sprintf("Finish wait for workload resources, --timeout time left %s", time.Until(deadline).String()))
}
}
}

if len(getDeploymentsError) > 0 {
errorMsg := fmt.Sprintf("Time out retrying after %s", getDeploymentsError)
if len(getWorkloadResourceError) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if len(getWorkloadResourceError) > 0 {
if getWorkloadResourceError != "" {

or, longer but might show the intention a bit better:

Suggested change
if len(getWorkloadResourceError) > 0 {
if errDeployment != nil || errDaemonSets != nil || errStatefulSets != nil

errorMsg := fmt.Sprintf("Time out retrying after %s", getWorkloadResourceError)
utils.LogError(errorMsg)
return errors.New(errorMsg)
}
if len(unavailableDeployments) > 0 {
errorMsg := "error unavailable deployments, timeout has expired, please consider increasing the timeout using the chart-verifier --timeout flag"
if len(unavailableWorkloadResources) > 0 {
errorMsg := "error unavailable workload resources, timeout has expired, please consider increasing the timeout using the chart-verifier --timeout flag"
utils.LogError(errorMsg)
return errors.New(errorMsg)
}
Expand Down Expand Up @@ -182,6 +212,22 @@ func getDeploymentsList(k Kubectl, context context.Context, namespace string, se
return list.Items, err
}

func getStatefulSetsList(k Kubectl, context context.Context, namespace string, selector string) ([]v1.StatefulSet, error) {
list, err := k.clientset.AppsV1().StatefulSets(namespace).List(context, metav1.ListOptions{LabelSelector: selector})
if err != nil {
return nil, err
}
return list.Items, err
}

func getDaemonSetsList(k Kubectl, context context.Context, namespace string, selector string) ([]v1.DaemonSet, error) {
list, err := k.clientset.AppsV1().DaemonSets(namespace).List(context, metav1.ListOptions{LabelSelector: selector})
if err != nil {
return nil, err
}
return list.Items, err
}

func GetLatestKubeVersion() string {
return latestKubeVersion.String()
}
Loading