Repository: knative/pkg
Pull request #1264 (2020-04-29 23:06:16)

populate the v1alpha1.Hostname from the URI as per the spec
As discovered as part of the move to v1beta1 reconcile work, the v1alpha1 down conversion is dropping the Hostname from the Status.
https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_eventing/3070/pull-knative-eventing-integration-tests/1255611066595938304

```
    --- FAIL: TestChannelStatus/TestChannelStatus-{Channel_messaging.knative.dev/v1alpha1} (2.34s)
        test_runner.go:95: namespace is : "v1alpha1-lhmqk"
        channel_status_test_helper.go:50: Running channel status conformance test with channel {"Channel" "messaging.knative.dev/v1alpha1"}
        channel_status_test_helper.go:54: Creating channel {Kind:Channel APIVersion:messaging.knative.dev/v1alpha1}-channel-req-labels
        creation.go:52: Creating channel &TypeMeta{Kind:Channel,APIVersion:messaging.knative.dev/v1alpha1,}-channel-req-labels
        channel_status_test_helper.go:78: No hostname found for {"Channel" "messaging.knative.dev/v1alpha1"}
```

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1264#" title="Author self-approved">vaikas</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/duck/OWNERS](https://github.com/knative/pkg/blob/master/apis/duck/OWNERS)~~ [vaikas]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-prow-robot: @vaikas: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | e84903dab1d94b61d52c92c06c987cd584e1bb3a | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1264/pull-knative-pkg-unit-tests/1255634624898404353) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1264). [Your PR dashboard](https://gubernator.knative.dev/pr/vaikas).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
vaikas: Spec bit:
```
Channel Status
v1alpha1
When the channel instance is ready to receive events status.address.hostname and status.address.url MUST be populated and status.addressable MUST be set to True.
```

https://github.com/knative/eventing/blob/master/docs/spec/channel.md#spec-requirements
knative-prow-robot: @vaikas: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | e84903dab1d94b61d52c92c06c987cd584e1bb3a | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1264/pull-knative-pkg-unit-tests/1255634624898404353) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1264). [Your PR dashboard](https://gubernator.knative.dev/pr/vaikas).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
vaikas: This failed?
--- FAIL: TestMemStatsMetrics (1.31s)

/test pull-knative-pkg-unit-tests

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1264#pullrequestreview-403108466" title="LGTM">grantr</a>*, *<a href="https://github.com/knative/pkg/pull/1264#" title="Author self-approved">vaikas</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/duck/OWNERS](https://github.com/knative/pkg/blob/master/apis/duck/OWNERS)~~ [vaikas]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
grantr: /hold

Holding if you want to address @vagababov's comment.
vaikas: /unhold
talked with Victor and he's fine with this going in this way.
Thanks!!

Patch:
diff --git a/apis/duck/v1alpha1/addressable_types.go b/apis/duck/v1alpha1/addressable_types.go
index 70c9b0135..328246f6f 100644
--- a/apis/duck/v1alpha1/addressable_types.go
+++ b/apis/duck/v1alpha1/addressable_types.go
@@ -106,9 +106,15 @@ func (a *Addressable) ConvertFrom(ctx context.Context, from apis.Convertible) er
 	switch source := from.(type) {
 	case *v1.Addressable:
 		a.URL = source.URL.DeepCopy()
+		if a.URL != nil {
+			a.Hostname = a.URL.Host
+		}
 		return nil
 	case *v1beta1.Addressable:
 		a.URL = source.URL.DeepCopy()
+		if a.URL != nil {
+			a.Hostname = a.URL.Host
+		}
 		return nil
 	default:
 		return fmt.Errorf("unknown version, got: %T", from)


Test patch:
diff --git a/apis/duck/v1alpha1/addressable_types_test.go b/apis/duck/v1alpha1/addressable_types_test.go
index 0daabf993..ad65a7cb7 100644
--- a/apis/duck/v1alpha1/addressable_types_test.go
+++ b/apis/duck/v1alpha1/addressable_types_test.go
@@ -30,45 +30,30 @@ func TestGetURL(t *testing.T) {
 	tests := []struct {
 		name string
 		addr Addressable
-		want apis.URL
+		want *apis.URL
 	}{{
 		name: "just hostname",
 		addr: Addressable{
 			Hostname: "foo.com",
 		},
-		want: apis.URL{
-			Scheme: "http",
-			Host:   "foo.com",
-		},
+		want: apis.HTTP("foo.com"),
 	}, {
 		name: "just url",
 		addr: Addressable{
 			Addressable: v1beta1.Addressable{
-				URL: &apis.URL{
-					Scheme: "https",
-					Host:   "bar.com",
-				},
+				URL: apis.HTTP("bar.com"),
 			},
 		},
-		want: apis.URL{
-			Scheme: "https",
-			Host:   "bar.com",
-		},
+		want: apis.HTTP("bar.com"),
 	}, {
 		name: "both fields",
 		addr: Addressable{
 			Hostname: "foo.bar.svc.cluster.local",
 			Addressable: v1beta1.Addressable{
-				URL: &apis.URL{
-					Scheme: "https",
-					Host:   "baz.com",
-				},
+				URL: apis.HTTPS("baz.com"),
 			},
 		},
-		want: apis.URL{
-			Scheme: "https",
-			Host:   "baz.com",
-		},
+		want: apis.HTTPS("baz.com"),
 	}}
 
 	for _, test := range tests {
@@ -93,11 +78,9 @@ func TestConversion(t *testing.T) {
 		name: "v1",
 		addr: &Addressable{
 			Addressable: v1beta1.Addressable{
-				URL: &apis.URL{
-					Scheme: "https",
-					Host:   "bar.com",
-				},
+				URL: apis.HTTP("bar.com"),
 			},
+			Hostname: "bar.com",
 		},
 		conv:        &v1.Addressable{},
 		wantErrUp:   false,
@@ -106,11 +89,9 @@ func TestConversion(t *testing.T) {
 		name: "v1beta1",
 		addr: &Addressable{
 			Addressable: v1beta1.Addressable{
-				URL: &apis.URL{
-					Scheme: "https",
-					Host:   "bar.com",
-				},
+				URL: apis.HTTP("bar.com"),
 			},
+			Hostname: "bar.com",
 		},
 		conv:        &v1beta1.Addressable{},
 		wantErrUp:   false,
@@ -119,10 +100,7 @@ func TestConversion(t *testing.T) {
 		name: "v1alpha1",
 		addr: &Addressable{
 			Addressable: v1beta1.Addressable{
-				URL: &apis.URL{
-					Scheme: "https",
-					Host:   "bar.com",
-				},
+				URL: apis.HTTPS("bar.com"),
 			},
 		},
 		conv:        &Addressable{},
@@ -186,10 +164,7 @@ func TestConvertTo(t *testing.T) {
 		},
 		conv: &v1beta1.Addressable{},
 		want: &v1beta1.Addressable{
-			URL: &apis.URL{
-				Scheme: "http",
-				Host:   "bar.com",
-			},
+			URL: apis.HTTP("bar.com"),
 		},
 		wantErrUp:   false,
 		wantErrDown: false,
@@ -207,10 +182,7 @@ func TestConvertTo(t *testing.T) {
 		},
 		conv: &v1.Addressable{},
 		want: &v1.Addressable{
-			URL: &apis.URL{
-				Scheme: "http",
-				Host:   "bar.com",
-			},
+			URL: apis.HTTP("bar.com"),
 		},
 		wantErrUp:   false,
 		wantErrDown: false,
@@ -233,3 +205,32 @@ func TestConvertTo(t *testing.T) {
 		})
 	}
 }
+
+func TestConvertFrom(t *testing.T) {
+	tests := []struct {
+		name string
+		in   apis.Convertible
+		want *Addressable
+	}{{
+		name: "v1beta1",
+		in:   &v1beta1.Addressable{URL: apis.HTTP("foo.example.com")},
+		want: &Addressable{Addressable: v1beta1.Addressable{URL: apis.HTTP("foo.example.com")}, Hostname: "foo.example.com"},
+	}, {
+		name: "v1",
+		in:   &v1.Addressable{URL: apis.HTTP("bar.example.com")},
+		want: &Addressable{Addressable: v1beta1.Addressable{URL: apis.HTTP("bar.example.com")}, Hostname: "bar.example.com"},
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := &Addressable{}
+			if err := got.ConvertFrom(context.Background(), test.in); err != nil {
+				t.Errorf("ConvertFrom() = %v", err)
+			}
+
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("roundtrip (-want, +got) = %v", diff)
+			}
+		})
+	}
+}